Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-10588] Diagnostics should support the %select{...}n specifier on strings #52988

Closed
beccadax opened this issue Apr 30, 2019 · 1 comment
Closed

[SR-10588] Diagnostics should support the %select{...}n specifier on strings #52988

beccadax opened this issue Apr 30, 2019 · 1 comment
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement

Comments

@beccadax
Copy link
Contributor

Previous ID SR-10588
Radar None
Original Reporter @beccadax
Type Improvement
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, DiagnosticsQoI, StarterBug
Assignee owenvoorhees (JIRA)
Priority Medium

md5: c7d0dceb508174072cd141bfdda6ebbe

Issue Description:

It's not uncommon to have some text that needs to be displayed before or after a string if it's present, but not if it's absent. For example, availability diagnostics can sometimes have a user-specified message that's emitted after the compiler's message; if there is one, it's shown after a colon and a space.

There are currently two ways we can handle this. One is to have two variants of the diagnostic:

ERROR(availability_decl_unavailable_rename, none,
      "%select{getter for |setter for |}0%1 has been "
      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
      "'%4'",
      (unsigned, DeclName, bool, unsigned, StringRef))

ERROR(availability_decl_unavailable_rename_msg, none,
      "%select{getter for |setter for |}0%1 has been "
      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
      "'%4': %5",
      (unsigned, DeclName, bool, unsigned, StringRef, StringRef))

The other is to add a boolean parameter saying whether the string should be included and then pass that parameter along with the string, like so:

ERROR(availability_decl_unavailable_rename, none,
      "%select{getter for |setter for |}0%1 has been "
      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
      "'%4'%select{|: %6}5",
      (unsigned, DeclName, bool, unsigned, StringRef, bool, StringRef))

It would be better to have one message that could handle both cases, perhaps like this:

ERROR(availability_decl_unavailable_rename, none,
      "%select{getter for |setter for |}0%1 has been "
      "%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
      "'%4'%select{|: %5}5",
      (unsigned, DeclName, bool, unsigned, StringRef, StringRef))

If you want to take a crack at this, look for a function called formatDiagnosticArgument() in DiagnosticEngine.cpp.
@swift-ci
Copy link
Collaborator

Comment by Owen Voorhees (JIRA)

Added in #26836

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@beccadax @swift-ci