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

enh(autodetect) tcl: improve autodetection #2865

Merged
merged 6 commits into from Feb 14, 2021

Conversation

joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Nov 15, 2020

Pull TCL auto-detect changes into their own PR for closer inspection

Changes

Checklist

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

@joshgoebel
Copy link
Member Author

Refer to thoughts here: #2745 (comment)

@joshgoebel joshgoebel self-assigned this Nov 15, 2020
@joshgoebel joshgoebel added this to the 10.5 milestone Nov 20, 2020
@joshgoebel
Copy link
Member Author

joshgoebel commented Dec 18, 2020

Screen Shot 2020-12-18 at 4 49 42 AM

Upon taking a closer look at this I'm not sure array index should be part of variable at all... line 2 there looks "correct" to me... with () being unhighlighted and 1 being highlighted as a number.

I'm less sure what the correct answer is when the whole expresion is wrapped in {}:

${ns::other::name(32)}

Just to make forward progress it wouldn't bother me if this is all a variable but when not enclosed in ${} then we only highlight the $blah part. IE:

Screen Shot 2020-12-18 at 4 55 30 AM

Are these all the same?

$name
${name}
${::name}
${name()}

@joshgoebel
Copy link
Member Author

Or perhaps include number inside ${}:

Screen Shot 2020-12-18 at 5 30 10 AM

@joshgoebel
Copy link
Member Author

joshgoebel commented Dec 19, 2020

I've pretty much lost the thread here (been too long) but I think the original idea was the make the rules more precise and less likely to match false-positives... plus the rules are pretty unusual as written now IMHO...

@joshgoebel joshgoebel removed this from the 10.5 milestone Dec 23, 2020
@allejo
Copy link
Member

allejo commented Dec 25, 2020

Sorry I'm just getting around to this one, what do you want to do this? Just classify everything as variables?

Or perhaps include number inside ${}:

Screen Shot 2020-12-18 at 5 30 10 AM

I think this one looks like it makes the most sense

@joshgoebel joshgoebel added the autoclose Flag things to future autoclose. label Jan 29, 2021
For languages with $ident and @Ident style variables this attempts
to prevent positives for $ident$ and @Ident@ type expressions, which
are likely something else entirely.

- Improves TCL variable detection to be more explicit
- Adds TCL variable detection tests
@joshgoebel joshgoebel merged commit 374a7e5 into highlightjs:master Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autoclose Flag things to future autoclose.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants