Handle the pattern field in ctags/readtags output#72
Handle the pattern field in ctags/readtags output#72dramforever wants to merge 12 commits intogediminasz:masterfrom
Conversation
2024f5c to
7581360
Compare
This comment was marked as outdated.
This comment was marked as outdated.
7581360 to
864cc04
Compare
864cc04 to
686a9fd
Compare
|
Hi, thank you for the contribution. I will try to have a look as soon as I can. |
686a9fd to
74d9a3d
Compare
|
Oh, CI is failing due to |
|
Hi, I've included an "Update dependencies" commit. Hope that works for you. By the way, sorry for the many updates after opening the PR. As mentioned earlier I only noticed I forgot to do the tests after opening this PR so there was more extra stuff than I expected to have submitted. |
No worries, been there myself :) |
dfd0b21 to
22e7f40
Compare
|
There, that should be enough coverage... |
The Uri class has a toString() method that is useful for storage and comparison. Implement it by converting Uri to an actual class. Use Uri.parse for constructing Uri objects everywhere.
These will be used in a future feature implementation. Stub those out for now to keep tests passing.
Some ctags files have a search pattern in the "tagaddress" field for robustness against file changes. This allows us to check if the file has changed and has caused the line to move, and possibly still find it based on the pattern. Implement this feature. Since this feature is a performance hit, add the ctags-companion.usePatternField option to allow toggling it off.
22e7f40 to
142d412
Compare
f81db6c to
1d8a501
Compare
1d8a501 to
62a7560
Compare
|
Sorry I'm taking my time with this PR but it's kinda big for a repo as small as this one and I try to be careful when adding new functionality... In docs it says:
Should we update the default |
|
The following command line illustrates the effect of -E option of readtags. $ printf 'the tile including \t tab\n----------------------------\n'
the tile including tab
----------------------------
$ printf 'the tile including \t tab\n----------------------------\n' > foo.rst
$ ./ctags -o - foo.rst | ./readtags -t - -l
the tile including tab foo.rst /^the tile including tab$/
$ ./ctags -o - foo.rst | ./readtags -t - -l -E
the tile including \t tab foo.rst /^the tile including tab$/ |
That's very understandable. Thank you :) for your review as well.
I hadn't changed it to preserve the current behavior, but it seems reasonable to me. But maybe it's out of scope for this PR? Notably, the pattern field is still unescaped, so the handling I wrote would still be needed even with |
Yes, let's keep it out of scope. I just checked and it is broken on master, so I'll have to look into that separately. |
|
I apologise @dramforever, you must have spent a lot of your precious time on this, but after a couple of weeks of marinating I decided to not take in this feature. When creating the original issue #59 I did not know about all of the intricacies. Turns out it's a non trivial change, and I don't know if I'll have the capacity to maintain it in the future. If you still have interest, I would suggest exploring alternative approaches:
|
|
Hi, this is understandable. I'm happy enough running a fork as well - your extension, by virtue of using |
Some ctags files have a search pattern in the "tagaddress" field for robustness against file changes. This allows us to check if the file has changed and has caused the line to move, and possibly still find it based on the pattern. Implement this feature.
Since this feature is a performance hit, add the
ctags-companion.usePatternField option to allow toggling it off.
This closes #59. There could still be minor problems, but I have been using this to browse Linux source code and it has worked reasonably well for me. Please take a look.