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

JavaScript: Improve support for contextual keywords as identifiers #3993

Merged
merged 3 commits into from
May 9, 2024

Conversation

b4n
Copy link
Member

@b4n b4n commented May 7, 2024

As the title says, better handle contextual keywords as function, property and method identifiers.

All these are probably fairly rare, but some did appear in e.g. a minified jQuery, leading to emitting null tags. Now, only the actually empty-named property it contains (""://…) leads to emitting a null tag.

This mostly builds up the get and set handling for all contextual keywords valid in these situations.

b4n added 3 commits May 8, 2024 00:23
`isKeyword()` doesn't check the token type, only the keyword it has, so
when resetting a keyword to an identifier, also mark it `KEYWORD_NONE`.
@b4n b4n requested review from jafl and masatake May 7, 2024 23:29
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.41%. Comparing base (07cbb8c) to head (eb830ef).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3993   +/-   ##
=======================================
  Coverage   85.40%   85.41%           
=======================================
  Files         235      235           
  Lines       56636    56639    +3     
=======================================
+ Hits        48371    48376    +5     
+ Misses       8265     8263    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@masatake
Copy link
Member

masatake commented May 8, 2024

About the new test case, adding args.ctags file with contents --sort=no makes expected.tags easier to read.

@b4n
Copy link
Member Author

b4n commented May 8, 2024

About the new test case, adding args.ctags file with contents --sort=no makes expected.tags easier to read.

I would agree in general, but I think that in the case of contextual.d it's pretty handy that it's sorted so you can easily see that there is basically 4 identical symbols of each name (but for the instances and class names, and the length property I could just get rid of as well).
If you prefer sorted though I'll add it not problem.

For the other test case sure, yet it's already sorted in the source so won't change a thing for now :)

@masatake
Copy link
Member

masatake commented May 9, 2024

@jafl I'm going to merge this as you approved. So we can work on the topic discussed at #3363.

@masatake masatake merged commit 141a8e0 into universal-ctags:master May 9, 2024
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants