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.

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

Import diff parser from geany #503

Merged

Conversation

masatake
Copy link
Member

No description provided.

} diffKind;

static kindOption DiffKinds [] = {
{ TRUE, 'f', "compared file", "compared files"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is "new file" now, this should probably rather be "modified file (m)", what do you think?

@masatake masatake force-pushed the import-diff-parser-from-geany branch from 6dc9be9 to 7630c31 Compare August 10, 2015 15:11
@masatake
Copy link
Member Author

@b4n, thank you for the great suggestions.
Updated.

@vhda
Copy link
Contributor

vhda commented Aug 10, 2015

Need to update the VS project files with entries for the added files.
We should really resolve #478 to have this feedback automatically.

@vhda
Copy link
Contributor

vhda commented Aug 10, 2015

Could you avoid using spaces in the long kind names? Could you instead use a single word in the long name and include a description containing the long name with space?
Apparently some tools might not be able to correctly parse extended information in the format modified file:path/to/file. This is true at least for Vim's Tagbar plugin.

@masatake
Copy link
Member Author

format.rst doesn't say about spaces in value slot of extra fields:

A tagfield has a name, a colon, and a value: "name:value".

* The name consist only out of alphabetical characters.  Upper and lower case
  are allowed.  Lower case is recommended.  Case matters ("kind:" and "Kind:
  are different tagfields).

* The value may be empty.
  It cannot contain a <Tab>.

  - When a value contains a "\\t", this stands for a <Tab>.
  - When a value contains a "\\r", this stands for a <CR>.
  - When a value contains a "\\n", this stands for a <NL>.
  - When a value contains a "\\\\", this stands for a single '\\' character.

  Other use of the backslash character is reserved for future expansion.
  Warning: When a tagfield value holds an MS-DOS file name, the backslashes
  must be doubled!

Actually ruby parser uses a space in the slot:

static kindOption RubyKinds [] = {
...
    { TRUE, 'F', "singleton method", "singleton methods" },

Once the rule, "don't use space in values in extra fields" is introduced, we have to force the same rule to the regex paresrs and xcmd parsers. So we must be careful here. Of course applications are important.

@majutsushi, can I ask you a question about tagbar?
@vhda wrote your Tagbar plugin doesn't allow spaces(not tabs) in kind fields.
Isn't it possible to relax the restriction?

@masatake
Copy link
Member Author

I will update the VS project file.

@masatake masatake force-pushed the import-diff-parser-from-geany branch from 7630c31 to 0f4b7e8 Compare August 11, 2015 05:49
@masatake
Copy link
Member Author

I see. It becomes issue when enabling scope information.

   {tagfield}   See below. 

A tagfield has a name, a colon, and a value: "name:value".
- The name consist only out of alphabetical characters.  Upper and lower case
  are allowed.  Lower case is recommended.  Case matters ("kind:" and "Kind:
  are different tagfields).

We should fix other parers. We should add Assertion to reject "alphabetical characters".
modifiedFile can be used.

@vhda
Copy link
Contributor

vhda commented Aug 14, 2015

Exactly. I think the other parsers do not have scope, so it's not a big issue.

@masatake
Copy link
Member Author

...diff parser can be fixed easily but what we should do about ruby parser?
About regex parsers, I have introduce assertion or ...

typedef struct sKindOption {
    boolean enabled;          /* are tags for kind enabled? */
    int letter;               /* kind letter */
    const char* name;         /* kind name */
    const char* description;  /* displayed in --help output */
} kindOption;

A value for a name field should be generated from the value for the associated description.
How about xcmd....

After making kind names in diff parser caml case, I will merge this.

@masatake masatake force-pushed the import-diff-parser-from-geany branch from 0f4b7e8 to 6e6383c Compare August 14, 2015 20:12
@masatake
Copy link
Member Author

I will rethink about 3 compeonets of a kind: letter, name, and description.

@masatake masatake self-assigned this Aug 22, 2015
@masatake
Copy link
Member Author

I research this area a bit. Following parsers and their kinds violates "alphabetical characters" rule.

Fortran
    block data
Java
    enum constant
OCaml
    Record field
Perl
    subroutine declaration
Ruby
    singleton method
Vera
    system header
    local header

Should I fix all? For following the rule, we have to convert them to camel case strings.
Do you want?

I myself think we can allow characters other than tab character. Instead we should write it in docs/format.rst. a tab should be rejected with Assert.

exuberant ctags already introduced a space in kind name:

static kindOption JavaKinds [] = {
    { TRUE,  'c', "class",         "classes"},
    { TRUE,  'e', "enum constant", "enum constants"},

@vhda, you suggest me to use "new, modified, deleted". However I would like to use nouns.
If people want something short, one should use --field=k(single letter).

I wonder why exuberant ctags violates the rules defined in FORMAT. As far as I know the same person worked on the both products...

I wanted to work only the implementation, ctags. However, we may have to work on FORMAT.

The description for 'm' kind is changed to "modified file".
(The name is suggested by @b4n.)

"*.diff" is removed from def->patterns; such file name is
haneled with "diff" extension.

Quoted from git diff --follow geany/tagmanager/ctags/diff.c:

    commit d69a153bb440bf5e27db6851fa291555be5f9d4b
    Author: Colomban Wendling <ban@herbesfolles.org>
    Date:   Tue May 8 22:14:29 2012 +0200

	Refactor tagmanager source files architecture

	Split ctags and tagmanager sources, as follows:

	tagmanager/ctags: the parsers, more or less upstream CTags;
	tagmanager/mio: local MIO library copy;
	tagmanager/src: actual tagmanager sources.

    commit 5ee037bfc00b34a316942f03c6b24fac70583005
    Author: Colomban Wendling <ban@herbesfolles.org>
    Date:   Fri Aug 19 13:12:53 2011 +0000

	Don't make tags for /dev/null in diff files but for the new file instead

	Based on a patch by Yang Hong, thanks.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@5893 ea778897-0a13-0410-b9d1-a72fbfd435f5

    commit 4cfedde35ad81555e65ba38cea0c354c7e844f0d
    Author: Colomban Wendling <ban@herbesfolles.org>
    Date:   Sun Mar 20 16:02:52 2011 +0000

	Fix a few warnings and style

	* Don't use strlen(..) > 0 or == 0, simply check the first character
	  against 0;
	* Fix a return without a value (my bad in last commit);
	* Fix storing a literal in a non-const string.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@5610 ea778897-0a13-0410-b9d1-a72fbfd435f5

    commit 746a8d5834117d967a1a57ca7c5d9b7de2bc0836
    Author: Nick Treleaven <nick.treleaven@btinternet.com>
    Date:   Fri Jul 10 15:57:12 2009 +0000

	Show relative paths in diff filename tags.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@3951 ea778897-0a13-0410-b9d1-a72fbfd435f5

    commit b7bfb2743a8de9c1457092934c1759c6882d65aa
    Author: Enrico Tröger <enrico.troeger@uvena.de>
    Date:   Wed Feb 27 13:17:29 2008 +0000

	Replace all C++-style comments with usual C-like multi-line comments.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@2287 ea778897-0a13-0410-b9d1-a72fbfd435f5

    commit 2da21eb5ccfb93fdc2991a8378ed1798c9ad6956
    Author: Enrico Tröger <enrico.troeger@uvena.de>
    Date:   Thu Oct 12 10:45:49 2006 +0000

	Fixed compiler warnings.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@886 ea778897-0a13-0410-b9d1-a72fbfd435f5

    commit adc721c5222dd52c11d4c50ab80eec661c68d5b5
    Author: Enrico Tröger <enrico.troeger@uvena.de>
    Date:   Wed Oct 11 19:45:40 2006 +0000

	Added simple parser for filetype Diff to create tags for each patched file in a diff file.

	git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@883 ea778897-0a13-0410-b9d1-a72fbfd435f5
If the original one of compared files is /dev/null,
use this new "new file(n)" kind.

A whitespace in the kind name is removed. In stead I use
camel case here. Suggested by @vhda.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Suggested by @vhda.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake masatake force-pushed the import-diff-parser-from-geany branch from 6e6383c to 24838f3 Compare August 28, 2015 19:21
@masatake
Copy link
Member Author

preservim/tagbar#288

I asked a question about this issue to the tagbar developer.

I recognized exuberant ctags itself violates the spec defined in FORMAT file.
So we have to refine the specification.

@masatake
Copy link
Member Author

I will merge my commits to reduce the issues.
I will reopen this issue after getting the comment from @majutsushi.

masatake added a commit that referenced this pull request Aug 29, 2015
@masatake masatake merged commit 0c18341 into universal-ctags:master Aug 29, 2015
@masatake masatake deleted the import-diff-parser-from-geany branch August 29, 2015 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants