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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

If present, use tag line number instead of search pattern. #229

Merged
merged 2 commits into from Sep 28, 2017

Conversation

Projects
None yet
5 participants
@segevfiner
Contributor

segevfiner commented Sep 18, 2017

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Line numbers are emmited by ctags, by default, in some cases. This is true for C preprocessor macros, for example. So properly handle them.

The user can request not to emit them (The --excmd argument to ctags, and short forms of it) but it's best for Atom to handle this correctly since this is part of the documented syntax of ctags.

Original by: @raphinesse (PR #187). This is an updated version of that PR to ES6 from CoffeeScript + Minor Changes. Hope you don't mind 馃槈.

Benefits

symbols-view will properly work with tags files that contain line number references. For example references to C/C++ macros.

Supersedes and closes #187

If present, use tag line number instead of search pattern.
Line numbers are emmited by ctags, by default, in some cases. This is
true for C preprocessor macros, for example.

Original by: @raphinesse (PR #187)
@raphinesse

This comment has been minimized.

Show comment
Hide comment
@raphinesse

raphinesse Sep 18, 2017

As long as my original authorship is mentioned in the commit or something like that, I'm completely fine with my code being recycled.

I just sincerely hope for you that this PR will not share the fate of my original one. Open source projects that do not honor contributions really annoy me.

raphinesse commented Sep 18, 2017

As long as my original authorship is mentioned in the commit or something like that, I'm completely fine with my code being recycled.

I just sincerely hope for you that this PR will not share the fate of my original one. Open source projects that do not honor contributions really annoy me.

@rsese

This comment has been minimized.

Show comment
Hide comment
@rsese

rsese Sep 19, 2017

Member

Sorry about your original pull request @raphinesse, we really do appreciate help from the community. Additionally, I wanted to let you know that we've tweaked our process to better track and bring PRs up for discussion so they're less likely to fall through the cracks. Thanks again for your contributions 馃檱

Member

rsese commented Sep 19, 2017

Sorry about your original pull request @raphinesse, we really do appreciate help from the community. Additionally, I wanted to let you know that we've tweaked our process to better track and bring PRs up for discussion so they're less likely to fall through the cracks. Thanks again for your contributions 馃檱

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Sep 19, 2017

Member

@segevfiner Can you look at the travis failure?

Member

Ben3eeE commented Sep 19, 2017

@segevfiner Can you look at the travis failure?

@raphinesse

This comment has been minimized.

Show comment
Hide comment
@raphinesse

raphinesse Sep 19, 2017

@rsese Glad to hear that. Looking forward to seeing this getting merged.

raphinesse commented Sep 19, 2017

@rsese Glad to hear that. Looking forward to seeing this getting merged.

@segevfiner segevfiner referenced this pull request Sep 19, 2017

Closed

Editor crash while debugging after doing a step over #15688

1 of 1 task complete
@segevfiner

This comment has been minimized.

Show comment
Hide comment
@segevfiner

segevfiner Sep 19, 2017

Contributor

@Ben3eeE Travis now passes.

Contributor

segevfiner commented Sep 19, 2017

@Ben3eeE Travis now passes.

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Sep 19, 2017

Member

@segevfiner Great 馃挴 I added this PR to the project board.

Member

Ben3eeE commented Sep 19, 2017

@segevfiner Great 馃挴 I added this PR to the project board.

segevfiner added a commit to segevfiner/symbols-view that referenced this pull request Sep 26, 2017

Support prefix regex patterns in tags files
Out of desperation due to atom#229,
I tried to use `ctags -N`. But ctags than outputs prefix regex patterns
for the macros instead of line numbers, which symbols-view also doesn't
handle.

In reality, those are really vi regex patterns. A truly proper
implementation will try to match them as regex, possibly transforming vi
regex constructs to JavaScript ones. As another note, it is also
possible to request backwards search patterns via `ctags -B` whice
symbols-view also won't handle but I don't think many people really do
that...

segevfiner added a commit to segevfiner/symbols-view that referenced this pull request Sep 26, 2017

Support prefix regex patterns in tags files
Out of desperation due to atom#229,
I tried to use `ctags -N`. But ctags than outputs prefix regex patterns
for the macros instead of line numbers, which symbols-view also doesn't
handle.

In reality, those are really vi regex patterns. A truly proper
implementation will try to match them as regex, possibly transforming vi
regex constructs to JavaScript ones. As another note, it is also
possible to request backwards search patterns via `ctags -B` which
symbols-view also won't handle but I don't think many people really do
that...
@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Sep 28, 2017

Contributor

馃挴 Great work @raphinesse and @segevfiner; thanks for the contribution!

Contributor

maxbrunsfeld commented Sep 28, 2017

馃挴 Great work @raphinesse and @segevfiner; thanks for the contribution!

@maxbrunsfeld maxbrunsfeld merged commit 88827df into atom:master Sep 28, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Sep 28, 2017

Contributor

This will go out in Atom 1.22.

Contributor

maxbrunsfeld commented Sep 28, 2017

This will go out in Atom 1.22.

@segevfiner segevfiner deleted the segevfiner:sf-ctags-line-numbers branch Sep 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment