Skip to content

Conversation

@SomeRandomiOSDev
Copy link
Contributor

  1. When a diagnostic has multiple fix its the annotated string didn't insert newlines between fix-its
  2. When a syntax has the same node offset as one of its children, replacing the child with a fix-it would always replace the outermost node

1. When a diagnostic has multiple fix its the annotated string didn't insert newlines between fix-its
2. When a syntax has the same node offset as one of its children, replacing the child with a fix-it would *always* replace the outermost node
Comment on lines +23 to +27
┬──────────────────────────
├─ ⚠️ This is the first diagnostic.
│ ✏️ This is the first fix-it.
│ ✏️ This is the second fix-it.
╰─ ℹ️ This is the second diagnostic, it's a note.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the fix in DiagnosticsFormatter.swift, this output took the following form:

       ┬──────────────────────────
       ├─ ⚠️ This is the first diagnostic.
         ✏️ This is the first fix-it.  ✏️ This is the second fix-it.╰─ ℹ️ This is the second diagnostic, it's a note.

@stephencelis
Copy link
Member

@SomeRandomiOSDev Thanks for the PR and fixes! Just the one thing about the OptionSet fix, since that's an example that we took from Apple and we may periodically fetch newer versions from them. I don't think it needs to hold up this PR, but if you wanna clean things up let me know!

@stephencelis
Copy link
Member

@SomeRandomiOSDev I think @Matejkob also brings up a good point, that this is really an upstream issue that will hopefully be fixed by Apple soon. Because of that, I'm inclined to think we should limit this PR to just fixing the formatting issue to minimize future churn when reintegrating code back from Apple in the future.

@SomeRandomiOSDev
Copy link
Contributor Author

@SomeRandomiOSDev Thanks for the PR and fixes! Just the one thing about the OptionSet fix, since that's an example that we took from Apple and we may periodically fetch newer versions from them. I don't think it needs to hold up this PR, but if you wanna clean things up let me know!

Makes sense, I'll push a change that moves this fix it to a different macro type to keep the OptionSet macro inline with the one in the swift-syntax repo.

@SomeRandomiOSDev
Copy link
Contributor Author

@stephencelis @Matejkob I pushed changes that removed the changes to types pulled upstream from swift-syntax and trimmed it down to changes related to this repo only.

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants