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.

Sign up for GitHub

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

Jump to bottom

PuppetManifest: class, definition and resource names with underscores create expected tags #1906

Merged
merged 2 commits into from
Oct 7, 2018
Merged

PuppetManifest: class, definition and resource names with underscores create expected tags #1906

merged 2 commits into from
Oct 7, 2018

Conversation

ahakanbaba
Copy link
Contributor

No description provided.

@ahakanbaba ahakanbaba mentioned this pull request Oct 6, 2018
Closed
@ahakanbaba ahakanbaba changed the title PuppetManifest: class definition names can contain underscores PuppetManifest: class, definition and variables names with underscores create expected tags Oct 6, 2018
@ahakanbaba ahakanbaba changed the title PuppetManifest: class, definition and variables names with underscores create expected tags PuppetManifest: class, definition and variable names with underscores create expected tags Oct 6, 2018
@coveralls
Copy link

coveralls commented Oct 6, 2018 •
edited

Coverage Status

Coverage remained the same at 84.882% when pulling b4cdacc on ahakanbaba:puppetManifest-class-definition-name-underscore into d080788 on universal-ctags:master.

@ahakanbaba ahakanbaba changed the title PuppetManifest: class, definition and variable names with underscores create expected tags PuppetManifest: class, definition and resource names with underscores create expected tags Oct 6, 2018
@ahakanbaba
Copy link
Contributor Author

ahakanbaba commented Oct 6, 2018 •
edited

@masatake I think I have addressed your previous comment from the previous PR.

@ahakanbaba
Copy link
Contributor Author

Does the optlib/puppetManifest.c file get autogenerated from the optlib/puppetManifest.ctags file ?

@masatake
Copy link
Member

masatake commented Oct 6, 2018

Thank you. Could you include the change of .c file to the commit in which you change the associated .ctags file. .c file is generated from .ctags file by optlib2c translator. I don't know how to run the script in the build time when compiling ctags with VC on Windows.

@masatake
Copy link
Member

masatake commented Oct 6, 2018

Oh, you already added the .c file. Thank you.

@masatake masatake self-assigned this Oct 6, 2018
@masatake
Copy link
Member

masatake commented Oct 6, 2018

Could you squash 9171923 and 3e6ac83 into one. As you know, mine ( 9171923 ) doesn't fix the original bug.
The test case in 9171923 is useful. However, the test case is derived from your original bug report.
So, remove my credit.

@masatake
Copy link
Member

masatake commented Oct 6, 2018

BTW, if you are interested in improving PuppetManifest parser more, could you look at following test cases
after reading TAG ENTRIES section of man/ctags.1 (or ctags.1.rst.in):

  • puppet-tag.b
  • puppet-tagged.b

I wonder what should be tagged for the input files.
What language objects do Puppet users expect ctags capture?
If you have ideas, please, open an issue for this topic.

masatake and others added 2 commits October 6, 2018 18:39
@masatake @ahakanbaba
PuppetManifest: class and definition names can have underscores.
6b59a92 
According to puppet docs: https://puppet.com/docs/puppet/5.3/lang_reserved.html#classes-and-defined-resource-types
calss or a definition name can contain an underscore anywhere except the
first char.
Also added tests.
A minor name change for a test directory
fixes universal-ctags#1901
@ahakanbaba
PuppetManifest: resource names can contain an underscore.
b4cdacc
@ahakanbaba ahakanbaba force-pushed the puppetManifest-class-definition-name-underscore branch from 3a62a02 to b4cdacc Compare October 7, 2018 01:40
@ahakanbaba
Copy link
Contributor Author

Could you squash 9171923 and 3e6ac83 into one. As you know, mine ( 9171923 ) doesn't fix the original bug.
The test case in 9171923 is useful. However, the test case is derived from your original bug report.
So, remove my credit.

@masatake Squashed .

@ahakanbaba
Copy link
Contributor Author

BTW, if you are interested in improving PuppetManifest parser more, could you look at following test cases
after reading TAG ENTRIES section of man/ctags.1 (or ctags.1.rst.in):

  • puppet-tag.b
  • puppet-tagged.b

I wonder what should be tagged for the input files.
What language objects do Puppet users expect ctags capture?
If you have ideas, please, open an issue for this topic.

I will spend more time on the puppet parser.
I have created #1905 and probably will create more.
After that I can spend time one these as well.

@masatake masatake merged commit a39e1de into universal-ctags:master Oct 7, 2018
@masatake
Copy link
Member

masatake commented Oct 7, 2018

Oh, I wrote wrong. This close "#1901", not "#1902".
...I cannot recover this mistake.

@masatake
Copy link
Member

masatake commented Oct 7, 2018

@ahakanbaba, thank you!

@ahakanbaba ahakanbaba deleted the puppetManifest-class-definition-name-underscore branch October 7, 2018 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@masatake masatake

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ahakanbaba @coveralls @masatake