preservim / tagbar

Vim plugin that displays tags in a window, ordered by scope
https://preservim.github.io/tagbar
Other
6.09k stars 484 forks source link

Setting custom ctagsbin causes wrong kinds to be applied #878

Closed char101 closed 1 month ago

char101 commented 1 month ago

Hi,

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

https://github.com/preservim/tagbar/blob/12edcb59449b335555652898f82dd6d5c59d519a/autoload/tagbar.vim#L300-L308

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.

https://github.com/preservim/tagbar/blob/12edcb59449b335555652898f82dd6d5c59d519a/autoload/tagbar.vim#L773-L785

I believe a better implementation would be

if has_key(userdef, 'ctagsbin') && userdef.ctagsbin =~# a:bin.'$'
char101 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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