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

CMake: new parser #1818

Merged
merged 1 commit into from
Aug 12, 2018
Merged

Conversation

hadrielk
Copy link
Contributor

As mentioned in issue #1816, this is a CMake file parser using the optlib multitable regex feature.

@coveralls
Copy link

coveralls commented Aug 11, 2018

Coverage Status

Coverage increased (+0.03%) to 84.433% when pulling 806f296 on hadrielk:hadriel/cmake-ctags into 9ffe430 on universal-ctags:master.

@masatake masatake self-assigned this Aug 11, 2018
</Project>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change for the end of file should not be added. git add -p may help you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. That's my editor being too smart for its own good. I'll fix it.

@masatake
Copy link
Member

Other than one minor point about the build file for windoes. everything looks good to me.
I made one pull request to you to support end field. My pull request supports scope, too. However, I'm not sure the scope filed makes sense in cmake syntax. If my pull request is acceptable, could you merge and push the changes here?
(hadrielk#1)

After visiting your profile page I know why you needs cmake parser; you are a wireshark hacker.
Uuniversal-ctags can offer more attractions to you, visit #1798 :-P

@masatake
Copy link
Member

( I run "make chop LANGUAGES=CMake", "make slap LANGUAGES=CMake", and "make fuzz LANGUAGES=CMake". All are passed.)

@masatake
Copy link
Member

The techniques used in the cmake.ctags are impressive. I have to consider applying them to my PuppetManifest parser.

@hadrielk
Copy link
Contributor Author

I'm not sure the scope filed makes sense in cmake syntax

There's no namespace concept in CMake - they do have a concept of "scope", but not in the sense that ctags would care I think (other than the end: field). Most of their scoping is based on directory information. Except variables (created with set()), which if set in a function/macro are in that function's scope only, unless they specify the PARENT_SCOPE argument.

Do you think I should try to add the scope for variable tags within functions/macros? (ie, use {scope=ref}) If so I'll have to also look for PARENT_SCOPE and if it's there then not set the scope reference.

#
# variable
#
--_mtable-regex-CMake=variable/([A-Za-z0-9_.-]+)([# \t\n\)])/\1/v/{tleave}{advanceTo=2start}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug: all these {advanceTo=2start} should be {_advanceTo=2start}

@masatake
Copy link
Member

Could you squash 08c4658 to acdd8 ?
Could you squash the changes for optlib/cmake.ctags in 66c62e0 to 729059f ?
Could you squash the changes for Units/parser-cmake.r/cmake-comments.d/expected.tags and Units/parser-cmake.r/cmake-comments.d/input.ctags in 66c62e0 to d3803b4 ?
Could you sqash the changes for optlib/cmake.c in 66c62e0 to acdd8 ?

@hadrielk
Copy link
Contributor Author

Would you mind if I just squashed all my changes down to one commit? It would be easier.

@masatake
Copy link
Member

Merging it one ("CMake: new parser") is o.k.

As mentioned in issue universal-ctags#1816, this is a CMake file parser using the optlib
multitable regex feature.
@hadrielk
Copy link
Contributor Author

Done.

@masatake masatake merged commit 12f3977 into universal-ctags:master Aug 12, 2018
@masatake
Copy link
Member

Thank you!

@hadrielk hadrielk deleted the hadriel/cmake-ctags branch August 13, 2018 03:05
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