fix(python) fix or conflicts with string highlighting
#3897
Conversation
Build Size ReportChanges to minified artifacts in 4 files changedTotal change +17 B View Changes
|
src/languages/python.js
Outdated
| @@ -176,7 +176,7 @@ export default function(hljs) { | |||
| contains: [ hljs.BACKSLASH_ESCAPE ], | |||
| variants: [ | |||
| { | |||
| begin: /([uU]|[bB]|[rR]|[bB][rR]|[rR][bB])?'''/, | |||
| begin: /(\W|^)([uU]|[bB]|[rR]|[bB][rR]|[rR][bB])?'''/, | |||
There was a problem hiding this comment.
Aren't you really just expressing "word boundary" here... and can't that be done much simpler using \b?
There was a problem hiding this comment.
I think it's doing the same thing here.
But it didn't come first to my mind.
Thanks for your advice and I will replace (\W|^) with it.
There was a problem hiding this comment.
@joshgoebel please take another look at my changes.
Many thanks for considering my request.
Build Size ReportChanges to minified artifacts in 3 files changedTotal change +4 B View Changes
|
|
We need to add some |
|
I have tested the code with |
|
Now, I have discovered the issue with this code. That's the right show I tried to solve it until now it didn't solve it. |
|
Hey @joshgoebel, what is your opinion I understand the test mockups and I think we should put some of them such as Thanks before any thing |
|
An example of every string type could work... I assume we'd put that in a test called |
MohamedAli00949
force-pushed
the
fix-python-string-highlighting
branch
from
b4e0be8 to
3e0f4ce
Compare
Build Size ReportChanges to minified artifacts in 4 files changedTotal change +27 B View Changes
|
|
Hi @joshgoebel, I have added some markup tests to test my changes and fixed selecting the char before the string prefix as a car such as highlighting the 'plus operator(+)' at the following example Many thanks for considering my request. |
|
In your test you have:
Is this valid, an |
I think it's invalid, and |
Build Size ReportChanges to minified artifacts in 5 files changedTotal change +30 B View Changes
|
|
What is an |
i think its was for testing if the three letters prefix will be highlighted or highlighting the right string. |
|
I meant is |
The If these tests weren't important or out of test context, I will delete them now. |
|
Hi @joshgoebel, Thanks in advance |
It's totally OK to include some false positives in the tests (with a comment to explain what purpose they are serving) - but any false positives should still be valid Python code. We only care about how well we highlight actual real-life valid Python. |
You convinced me, thanks ❤ |
Build Size ReportChanges to minified artifacts in 5 files changedTotal change +30 B View Changes
|
MohamedAli00949
force-pushed
the
fix-python-string-highlighting
branch
from
603e1e6 to
2028286
Compare
Build Size ReportChanges to minified artifacts in 2 files changedTotal change +25 B View Changes
|
I have committed the right tests now, feel free to review and give me your good advice. |
|
Hi @joshgoebel, Any news about reviewing the changes? |
|
Hi @joshgoebel, If I have removed some of the test commits and collected the testing at one commit would that be best? |
|
If we just match |
Build Size ReportChanges to minified artifacts in 2 files changedTotal change +19 B View Changes
|
Firstly, I am sorry for replying late, I didn't see your comment until now, and thanks for your changes. |
joshgoebel
force-pushed
the
fix-python-string-highlighting
branch
from
658a345 to
4a1eddd
Compare
Build Size ReportChanges to minified artifacts in 5 files changedTotal change +17 B View Changes
|
|
Please add a changelog entry then and we should be good to go. |
Build Size ReportChanges to minified artifacts in 5 files changedTotal change +17 B View Changes
|
joshgoebel
changed the title
or conflicts with string highlighting
Build Size ReportChanges to minified artifacts in 5 files changedTotal change +23 B View Changes
|
I have fixed the error of highlighting the reserved words such as 'or' in the below code.
That should be in the below image.
Not as this highlight.
That happens because of the string prefixes (f, r, u, and b) or (F, R, U, and B).
This issue has been described at #3798
Changes
I have found that happens from string variants begin RegExes with the prefix
(\W|^).Checklist
(\W|^).