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

Setting custom ctagsbin causes wrong kinds to be applied #878

Closed
char101 opened this issue May 18, 2024 · 9 comments · Fixed by #881
Closed

Setting custom ctagsbin causes wrong kinds to be applied #878

char101 opened this issue May 18, 2024 · 9 comments · Fixed by #881

Comments

@char101
Copy link

char101 commented May 18, 2024

Hi,

In these lines, if the return value of s:CheckFTCtags is not empty, then the kinds for jsctags is applied to type javascript

tagbar/autoload/tagbar.vim

Lines 300 to 308 in 12edcb5

let jsctags = s:CheckFTCtags('jsctags', 'javascript')
if jsctags !=# ''
call tagbar#debug#log('Detected jsctags, overriding typedef')
let type_javascript = tagbar#prototypes#typeinfo#new()
let type_javascript.ctagstype = 'javascript'
let type_javascript.kinds = [
\ {'short' : 'v', 'long' : 'variables', 'fold' : 0, 'stl' : 0},
\ {'short' : 'f', 'long' : 'functions', 'fold' : 0, 'stl' : 1}
\ ]

But s:CheckFTCtags also returns non-empty string if ctagsbin is set, while ctagsbin might not be jsctags, thus the wrong kind of kinds mapping is applied, in this case, f become namespace instead of function.

tagbar/autoload/tagbar.vim

Lines 773 to 785 in 12edcb5

function! s:CheckFTCtags(bin, ftype) abort
if executable(a:bin)
return a:bin
endif
if exists('g:tagbar_type_' . a:ftype)
let userdef = g:tagbar_type_{a:ftype}
if has_key(userdef, 'ctagsbin')
return userdef.ctagsbin
else
return ''
endif
endif

I believe a better implementation would be

if has_key(userdef, 'ctagsbin') && userdef.ctagsbin =~# a:bin.'$'
@char101
Copy link
Author

char101 commented May 18, 2024

My custom ctagsbin is this deno script using ts-morph that works with typescript and javascript

import { Project, ConstructorDeclaration, MethodDeclaration, VariableDeclarationKind } from 'https://deno.land/x/ts_morph/mod.ts';

class Main {
  constructor(filename) {
    this.filename = filename;

    const project = new Project();
    const source = project.addSourceFileAtPath(filename);

    // global variables
    for (const vs of source.getVariableStatements()) {
      const isConstant = vs.getDeclarationKind() === VariableDeclarationKind.Const;
      for (const vd of vs.getDeclarations()) {
        this.tag(vd, isConstant ? 'C' : 'v');
      }
    }

    // global functions
    for (const fn of source.getFunctions()) {
      this.tag(fn, 'f');
    }

    // classes
    for (const cl of source.getClasses()) {
      const parent = this.tag(cl, 'c');
      for (const co of cl.getConstructors()) {
        this.tag(co, 'm', parent);
      }
      for (const mt of cl.getMethods()) {
        this.tag(mt, 'm', parent);
      }
    }
  }

  tag(node, kind, parent = undefined) {
    const name = node instanceof ConstructorDeclaration ? '@constructor' : node.getName() || '<Anonymous>';

    const fields = [name, this.filename, '/^/;"', kind, `line:${node.getStartLineNumber()}`];

    if (parent) {
      fields.push(parent);
    }

    if (node instanceof MethodDeclaration) {
      fields.push(`access:${node.getScope()}`);
    }

    console.log(fields.join('\t'));

    if (kind == 'c') {
      return `class:${name}`;
    }
  }
}

new Main(Deno.args[0]);

@raven42
Copy link
Collaborator

raven42 commented May 29, 2024

@char101
Hmm... so looking at this, I'm not sure your proposal would necessarily be ideal. It does look like there is an issue here as the current behavior is assuming that if the user has a custom defined ctagsbin, that it outputs something compatible with the other tagging utility (jsctags / gotags / etc). However your proposal now make the opposite assumption that if a user defined ctagsbin is defined for javascript, then it must NOT be compatible with jsctags unless it is specifically called the same thing.

I think we might be better adding a specific flag to the custom definition to indicate it is not jsctags compatible. To avoid issues with current deployments, we may want to continue with the assumption that a custom defined one WILL be compatible with jsctags, but then allow for a flag to indicate that it is not compatible to help in your situation.

Perhaps a solution like this?

diff --git a/autoload/tagbar.vim b/autoload/tagbar.vim
index de8b4b3..f5e2c72 100644
--- a/autoload/tagbar.vim
+++ b/autoload/tagbar.vim
@@ -778,7 +778,13 @@ function! s:CheckFTCtags(bin, ftype) abort
     if exists('g:tagbar_type_' . a:ftype)
         let userdef = g:tagbar_type_{a:ftype}
         if has_key(userdef, 'ctagsbin')
-            return userdef.ctagsbin
+            if !has_key(userdef, 'ctagscompatibility')
+                return userdef.ctagsbin
+            elseif userdef.ctagscompatibility ==# a:bin
+                return userdef.ctagsbin
+            else
+                return ''
+            endif
         else
             return ''
         endif

@char101
Copy link
Author

char101 commented May 30, 2024

I find it a bit weird though, the standard is ctags, isn't it?.

If there is a custom command or if I were to create a custom command, I would follow ctags kind names instead of jsctags kind names. Why would I need to follow jsctags kind names. Not to mention jsctags has not been updated for 6 years.

Since universal ctags now supports javascript, shouldn't jsctags itself marked as obsolete?

@raven42
Copy link
Collaborator

raven42 commented May 31, 2024

We may be able to do that. However it is really hard to guess how many people may be using the plugin as is right now with jsctags and how many people a change like that might affect. We are still continuing to use exuberant ctags as well, though not adding new feature support for it for a similar reason. The general model I've been following in any updates at least is to avoid breaking existing functionality.

If we get a strong agreement that we can remove support for jsctags, then we could definitely look at doing something like that. However I've mostly been volunteering my time to help maintain this plugin, but I don't really have time to do large updates or major reworks.

@raven42
Copy link
Collaborator

raven42 commented May 31, 2024

I've added a poll #880 to see if we can get any feedback to see if jsctags is used at all. Maybe from this we can get a feel for if it can be removed.

@char101
Copy link
Author

char101 commented Jun 1, 2024

Thank you for adding the poll.

Is it possible that if a custom ctagsbin is given and custom kinds is given (in g:tagbar_type_[filetype]) then the custom kinds is not overriden?

I think this might keep the compatiblity while not forcing the kinds to follow jsctags (or other custom ctags).

For example, for typescript I have this definition

vim9script
g:tagbar_type_typescript = {
	'ctagsbin': 'deno.exe',
	'ctagsargs': 'run --allow-net --allow-read ' .. $VIM  .. '/bin/tstags.js',
	'kinds': [
		'C:constants',
		'v:global variables',
		'f:functions',
		'c:classes',
		'm:methods'
	]
}

@raven42
Copy link
Collaborator

raven42 commented Jun 5, 2024

Yes I think that could be a good idea. Sorry I haven't had time to implement this yet. Looks like no one else has looked at the poll. I'll try to see if I can get something for this together and validated in the next few days.

@raven42
Copy link
Collaborator

raven42 commented Jun 5, 2024

Ok, give this a try and let me know how it works for you. The idea is if the user has a kinds defined in the user definition, then we will not override anything. Else if there is no kinds defined, it will check if the ctagsbin is there, and then override the kinds based assuming the same behavior as previous code.

diff --git a/autoload/tagbar.vim b/autoload/tagbar.vim
index de8b4b3..a85c122 100644
--- a/autoload/tagbar.vim
+++ b/autoload/tagbar.vim
@@ -771,19 +771,18 @@ endfunction
 
 " s:CheckFTCtags() {{{2
 function! s:CheckFTCtags(bin, ftype) abort
-    if executable(a:bin)
-        return a:bin
-    endif
-
     if exists('g:tagbar_type_' . a:ftype)
         let userdef = g:tagbar_type_{a:ftype}
-        if has_key(userdef, 'ctagsbin')
-            return userdef.ctagsbin
-        else
+        if has_key(userdef, 'kinds')
             return ''
+        elseif has_key(userdef, 'ctagsbin')
+            return userdef.ctagsbin
         endif
     endif
 
+    if executable(a:bin)
+        return a:bin
+    endif
     return ''
 endfunction
 

@char101
Copy link
Author

char101 commented Jun 5, 2024

I have tested it and it seems to work, thank you.

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 a pull request may close this issue.

2 participants