-
Notifications
You must be signed in to change notification settings - Fork 620
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
Add nix language parser #4020
base: master
Are you sure you want to change the base?
Add nix language parser #4020
Conversation
To docs/news/HEAD.rst, please add:
|
Thank you. |
4745a10
to
7258205
Compare
I updated the commit with your comments. Next I'll work on expanding the test case to actually test real-world nix code: I'll make a small library of functions and a couple builders with different formatting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have to put "NixParser" here:
Line 135 in 8976ec3
MyrddinParser, \ |
After doing "make", you can run the test cases with "make units LANGUAGES=Nix".
2838f80
to
6e0ea01
Compare
I added some better test nix code, I tried to look at some of my personal nix code and copy a few formatting patterns that I use, which is similar to the style you see in upstream nixpkgs afaik. So I would expect this optlib to work reasonably well everywhere. I thought I would need a multiline regex for the Anyway sorry for taking so long to finish this. But I've been using this optlib in my personal code, which runs over nixpkgs and indeed finds most definitions I need, and my work code, which has thousands of lines of custom nix and it seems to work more often than not. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4020 +/- ##
==========================================
- Coverage 85.43% 85.39% -0.04%
==========================================
Files 235 236 +1
Lines 56727 56750 +23
==========================================
Hits 48462 48462
- Misses 8265 8288 +23 ☔ View full report in Codecov by Sentry. |
Could you rebase your chnages on the latest code? |
--map-Nix=+.nix | ||
|
||
# Index packages when we see "name = <tag>" or "pname = <tag>" | ||
--kinddef-Nix=p,package,package definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as reading the test case, package
is not tested at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doh, good catch, i fixed the regex here
optlib/nix.ctags
Outdated
--kinddef-Nix=f,function,function definition | ||
--regex-Nix=/(\S+)\s*=\s+\w+:/\1/f/ | ||
|
||
# Attrs definitions just have =, but only index if they have >=4 chars, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for rejecting short names looks weak.
One of the mottos of Universal Ctags design is to "prepare raw information to ctags client tools like vim when ctags doesn't know what's incorrect."
Rejecting can be done in a client tool reading the tags file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, removed that part
The I don't know the Nix language at all. However, I guess you may want to fill out the scope fields for tag entries. Without it, a client tool like Vim cannot draw a tree, which is useful for navigation. struct point {
int x;
int y;
}; For the input of C language, ctags can emit the following tags:
From the tags output,
The current nix parser doesn't extract
Too many tags may not be a problem if the scope fields are filled out well. However, these improvements can be made after merging this pull request. |
Ah, scoped tags is a very good idea. It's something I would need if I want to |
This is a work in progress.Okay I think this is in good shape now, seems to work well in my usage and passes unit tests.