-
Notifications
You must be signed in to change notification settings - Fork 740
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
Improve Swift attribute formatting #806
Conversation
Makes sense to me! I remember changing it to the way it is now for some reason… Maybe not a good reason — I don't remember. I'd just suggest taking a look at blame/file history to see if there isn't something worth considering. |
OK - thanks! The previous changes were about not hard-coding the attribute names, nothing in conflict with this one. So will leave the PR as-is. |
Rebased! |
@johnfairh @pyrmont Is this PR still good to go? Looks like a good change for the lexer. |
I live in hope :) I know they're working through them. |
Great, was checking the PR was up-to-date and ready to go! |
12d2292
to
107305f
Compare
@johnfairh Apologies for force pushing to your fork—there have been a few more changes since your last rebase and I wasn't able to get the test suite to pass anymore. This all looks good to me. I'm just waiting for Travis to finish and then will merge in :) |
@johnfairh Sorry this took so long and thank you for being so patient. It's great to have Rouge now highlight this in a more readable way :) |
No worries, thanks for reviving the project. |
Swift @available attributes can go wrong:
The rparen inside the quoted string is accidentally ending the @available( and messing up what follows.
I also feel that treating the entire thing as a single token looks poor because of the internal structure. So this PR just allows 'normal' lexing of the part in parens giving e.g.:
and
The majority of attributes do not have args and are unaffected.
(This is similar in approach to Apple's default format:
)
This change also deals with whitespace + comments inside the attr args / between attr and paren.
If there is a reason why the entire thing should be lexed as a single token then I'd be happy to change this PR to fix just the rparen-matching issue!
// cc @radex who touched this last for any thoughts.