fixed hex decimals on C #4094
fixed hex decimals on C #4094
Conversation
src/languages/c.js
Outdated
| ], | ||
| relevance: 0 | ||
| }; | ||
| const NUMBERS = { |
There was a problem hiding this comment.
Tabbing/spacing here should not have changed...
There was a problem hiding this comment.
i have fixed the indentation in the new commits
|
Also we really should have some tests here to validate both the old behavior and confirm the new behavior. Could we perhaps pull over the |
@joshgoebel i have also come up with a new test and expect files for number literals in C since all C++ number literals cannot be directly ported over due to c(gcc) not having digit seprators |
|
i have also noticed some keywords in the C keywords file that dont belong there for e.g -'auto' these work in c++ and not c. |
Please make sure your knowledge is 100% up-to-date before offering to start removing keywords. |
More tests would be welcome, but I don't think we need a separate issue for "add more tests to C"... but perhaps a meta-issue could be good - it would just be a placeholder for all grammars tagged with "good for beginners". Many grammars could use additional tests. |
src/languages/c.js
Outdated
| { begin: '\\b(0b[01\']+)' }, | ||
| { begin: '(-?)\\b([\\d\']+(\\.[\\d\']*)?|\\.[\\d\']+)((ll|LL|l|L)(u|U)?|(u|U)(ll|LL|l|L)?|f|F|b|B)' }, | ||
| { begin: '(-?)(0[xX][a-fA-F0-9]+(?:\'[a-fA-F0-9]+)*(?:\\.[a-fA-F0-9]*(?:\'[a-fA-F0-9]*)*)?(?:[pP][-+]?[0-9]+)?)' }, | ||
| { begin: '(-?)\\b\\d+(?:\'\\d+)*(?:\\.\\d*(?:\'\\d*)*)?(?:[eE][-+]?\\d+)?' } |
There was a problem hiding this comment.
Now if we could please just switch to match vs begin and literal regex: /.../ (vs strings)... then this is good to go I think. I'm also happy to do this cleanup myself, just might be a little while.
There was a problem hiding this comment.
@joshgoebel i've changed the match to begin , i also managed to correct for a issue in detecting hex longs that went previously undetected.
please review if any changes are needed
src/languages/c.js
Outdated
| ], | ||
| { match: /\b(0b[01']+)/ }, | ||
| { match: /(-?)\b([\d']+(\.[\d']*)?|\.[\d']+)((ll|LL|l|L)(u|U)?|(u|U)(ll|LL|l|L)?|f|F|b|B)/ }, | ||
| { match: /(-?)(0[xX][a-fA-F0-9]+(?:'[a-fA-F0-9]+)*(?:\.[a-fA-F0-9]*(?:'[a-fA-F0-9]*)*)?(?:[pP][-+]?[0-9]+)?(l|L)?(u|U)?)/ }, |
There was a problem hiding this comment.
Do we want need \b here also?
There was a problem hiding this comment.
i did see a redundant \b and ive removed it
There was a problem hiding this comment.
I thought these were needed to prevent false positives... like say g2324 where the 234 would be identified as a number without the \b, no?
There was a problem hiding this comment.
the current behavior seems to work fine but ig we can do with a \b just to be sure but wouldnt that take a performance hit. should i add \b just to be sure or is it fine ?
There was a problem hiding this comment.
An example of false positives, wouldn't these be valid variable names? The \b is needed to prevent this.
wouldnt that take a performance hit.
Not really, no - I don't think so - but we don't really talk about regex performance here outside of security concerns (run-away exponential regex, etc).
There was a problem hiding this comment.
should i add \b
Yep. I think we need it.
There was a problem hiding this comment.
@joshgoebel ive fixed the false positive issue please review changes
add \b for last NUMBERS regex
Build Size ReportChanges to minified artifacts in 3 files changedTotal change +86 B View Changes
|
Resolves #4065
Changes
Changed the regex for hex numbers to accomdate decimal numbers
(first pr pls be nice 😁😁)
(https://github.com/highlightjs/highlight.js/issues/4065)
Checklist
Checklist