Create tags for constant definitions

[ragol]

A PHP project can have a lot of constants defined. This change adds a flag to ctags so that ctags create tags for constant definitions.

[greg0ire]

Github says "We can’t automatically merge this pull request.", probably because of this commit (it conflicts with your commit)… it shows some users will want cdfiv, while other might prefer cfi. I think could be stored in a configuration variable, which should default to cfi for backwards compatibility-reasons. Would you like to give this a try ? If not, I'll take care of it.

[ragol]

I have made tag kinds configurable as you suggested.

When doing so I had to remove "-e" from shebang since it was causing the script to abort if the new option is not set. I did not find any other place which should be affected by this change.

[greg0ire]

Nice job! But it looks like the tests fail :

ASSERT:$indexMe was not found here : !_TAG_FILE_FORMAT  2       /extended format; --format=1 will not append ;" to lines/
!_TAG_FILE_SORTED       1       /0=unsorted, 1=sorted, 2=foldcase/
!_TAG_PROGRAM_AUTHOR    Darren Hiebert  /dhiebert@users.sourceforge.net/
!_TAG_PROGRAM_NAME      Exuberant Ctags //
!_TAG_PROGRAM_URL       http://ctags.sourceforge.net    /official site/
!_TAG_PROGRAM_VERSION   Development     //
testTagsFileWorksWithSymfony2
ASSERT:$indexMe was not found here : !_TAG_FILE_FORMAT  2       /extended format; --format=1 will not append ;" to lines/
!_TAG_FILE_SORTED       1       /0=unsorted, 1=sorted, 2=foldcase/
!_TAG_PROGRAM_AUTHOR    Darren Hiebert  /dhiebert@users.sourceforge.net/
!_TAG_PROGRAM_NAME      Exuberant Ctags //
!_TAG_PROGRAM_URL       http://ctags.sourceforge.net    /official site/
!_TAG_PROGRAM_VERSION   Development 

[greg0ire]

The test that fails is ./tests/php/ctags_test.sh

[greg0ire]

Plus, you should rebase your branch on the latest master, there have been many changes since then.

[greg0ire]

Also, could you document your option in the README.md file ?

[ragol]

Damn, I always update all my repos. This the only one that I forgot. I'll merge master into my branch. Rebasing is not an option any more since I made my changes public.

[greg0ire]

I know what Linus says, but here I think rebasing is an option : once your changes get merged in my repo, you're probably going to delete your branch right ? Plus, you can still create a new branch that would have your commits rebased on master, and keep this branch unchanged if it really bothers you. This rule is really important for origin repositories with many people cloning them.

[greg0ire]

git checkout tag-constants && git checkout -b new_name && git rebase -i master should do the trick. I added -i because you may want to squash some commits together : there's not point in updating the README separately or fixing the tests separately.

[greg0ire]

But I'm probably the only person who got your branch (for fixing the tests), so…

[ragol]

Okay, I'll try rebasing. :smile:

[greg0ire]

Great! If you can, please also add a unit test, I'd like this code to be rock solid. Something like this :

testTagsFileOptions()
{
    git config hooks.php-ctags.tag-kinds cfi # do not index variables
    echo '<?php $variable = 42;' > foo.php
    git add foo.php
    git commit --quiet --message "foo file"
    sleep 1 # ctags is run in the background. Wait for it.
    assertTrue 'The tags file was not generated' "[ -f .git/tags ]"
    assertFalse "\$variable was found" "grep variable .git/tags"

    git config hooks.php-ctags.tag-kinds cfiv # index variables
    git commit --amend
    sleep 1 # ctags is run in the background. Wait for it.
    assertTrue "\$variable was not found" "grep variable .git/tags"
}

[ragol]

My version of ctags supports

• c classes
• d constant definitions
• f functions
• i interfaces
• l local variables [off]
• n namespaces
• t traits
• v variables

If you want to have it rock solid then I'd have to add a test for every flag taking into account locally installed ctags. That could be overkill, IMHO.

[greg0ire]

If you were doing that, you would be mainly testing ctags, not your code. The goal of your code is to pass an option from a configuration file to ctag, and a good way to test this is to test whether the index file changes when you change the option. That's roughly what the test I'm suggesting does : it does not test every combination, but checks whether a change in the git config results in a change of the generated index.

[greg0ire]

I edited my code snippet because I forgot a name replacement from indexMe to variable

[greg0ire]

But if you don't feel like writing the tests which can be quite hard / boring, just say it and I'll take care of it :wink:

[ragol]

That's why I said it would be overkill. Anyway, I've fixed your test and made it non-interactive (--amend opened an editor for commit message).

Also, you mentioned above that a test failed. I can't reproduce the failure.

[greg0ire]

You can't reproduce it because you merged my PR, that fixes it I think, don't you ?

[ragol]

Seems like it.

[ragol]

I created a new branch configurable-tag-kinds as you suggested. Should I create a new pull request?

[ragol]

I've changed the call to shunit2 to the one found in $PATH. If your version, which is in ~/src/shunit2/shunit2, is in $PATH it should just work. If not feel free to ignore this commit. :smile:

[greg0ire]

Yeah, I think you need to create a new PR.

[ragol]

Created a new pull request.