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

Update handling of Python decorators #2811

Merged
merged 5 commits into from Oct 31, 2020
Merged

Update handling of Python decorators #2811

merged 5 commits into from Oct 31, 2020

Conversation

textbook
Copy link
Contributor

@textbook textbook commented Oct 31, 2020

Resolves #2804.

Changes

Modifies the meta matching for Python code to handle decorators more accurately. This allows e.g. comments to be correctly formatted in lines starting with @.

  • Python 3.8 and earlier. Before PEP614 decorators had to be a dotted name.
  • Python 3.9 and later. Decorators can be any expression.

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md
  • Added myself to AUTHORS.txt, under Contributors

@textbook
Copy link
Contributor Author

textbook commented Oct 31, 2020

Playing a bit more with it, PEP614 makes this all-but impossible in the general case. Either comments are counted as part of the decorator, you have to implement arbitrary expression parsing in regex, or some of the decorator gets missed out.

Some examples below comparing pre- and post-PR Highlight.js as well as GitHub's formatting:

On GitHub:

@commented  # here
def func():
    pass

@py38.style
def func():
    pass

@py[3.9].style
def func():
    pass

@2 + 2 == 5
def func():
    pass

In Highlight.js without this PR:

Screenshot 2020-10-31 at 16 49 06

In Highlight.js with this PR:

Screenshot 2020-10-31 at 16 49 40

Notes:

  • Pre-PR Highlight.js is very greedy, the whole line gets meta'd
  • GitHub is much more conservative, either the whole thing is a decorator or it isn't, but comments are handled correctly
  • Post-PR Highlight.js is somewhere in the middle, taking as much as it can under the pre-PEP614 rules and letting the rest be formatted by other rules

Questions:

  • Is that an improvement overall?
  • Should the @ be meta in the last example?

@joshgoebel
Copy link
Member

joshgoebel commented Oct 31, 2020

Ok, let's forget about what's technically possible and look at how people typically use this instead. Every single example on the PEP614 page is a contiguous string of characters... What about:

@[contiguous characters] ... still meta ... (allow comments here)   $(the end)

So comments would be possible anywhere after the contiguous string of characters was broken... If you got really creative you could even allow for ,[space]... so that a list of arguments didn't break the idea "contiguous characters"... but we're forgetting the other formatting... if a number or string is included here, do we not want to highlight those separately?

@surround_with("#", repeat=3)

Should this ALL be a single color or should the string and numeric be matched as such?

@joshgoebel
Copy link
Member

Or (if we aren't highlighting) could we just add STRING inside contains (but remove it's className so it's not highlighted? Then the # is eaten by string and not seen as a comment... and If we ARE highlighting strings then this problem should resolve itself since the # is inside a string... so seems we first need to figure that out.

@textbook
Copy link
Contributor Author

Yep, that was the trick - when I'd experimented with ending on a # earlier I hadn't been using contains. I can ignore what the actual meta expression is, and just do end: /(?=#)|$/. Examples below.


Pre-PR:

Screenshot 2020-10-31 at 19 38 37

GitHub:

@foo
def bar():
    pass

@foo  # bar
def baz():
    pass

@foo.bar.baz
def qux():
    pass

@surround_with("#", repeat=3)
def text():
    return "hi!"

@py38.style
def func():
    pass

@py["3.9"].style
def func():
    pass

@py[3.9].style
def func():
    pass

@2 + 2 == 5
def func():
    pass

Post-PR:

Screenshot 2020-10-31 at 19 36 47

@joshgoebel
Copy link
Member

Ok, so the mode nesting solves the issue with comments, but now what about visually... do we want to see them as meta or styled as they are in any other code (like how you have it now).

@textbook
Copy link
Contributor Author

I like it with the literal strings and numbers with the non-meta styling, as GitHub does it on the surround_with example, but happy to go with the community view on that.

@textbook textbook marked this pull request as ready for review October 31, 2020 19:48
@joshgoebel
Copy link
Member

I like it with the literal strings and numbers with the non-meta styling, as GitHub does it on the surround_with example, but happy to go with the community view on that.

I'm happy with what we have now unless you want to go poll some other peoples...

@textbook
Copy link
Contributor Author

I'm happy with what we have now...

Me too! Let's ship it, the visuals can always be updated later if desired.

@joshgoebel joshgoebel merged commit cf63d2f into highlightjs:master Oct 31, 2020
@joshgoebel joshgoebel added the hacktoberfest-accepted Hacktoberfest label Oct 31, 2020
@textbook textbook deleted the python-decorators branch October 31, 2020 21:04
@joshgoebel
Copy link
Member

If you're looking for more fun with Python: #2633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(python) Decorator syntax breaks comment formatting
2 participants