rodjek / vim-puppet

Puppet niceties for your Vim setup
Apache License 2.0
500 stars 137 forks source link

Jump to tag with CTRL-] no longer works #140

Closed saj closed 2 years ago

saj commented 2 years ago

bisect says this was broken by https://github.com/rodjek/vim-puppet/pull/137. The related commentary would suggest that change is unlikely to be reverted. I file this report in the hope that it might offer some rationale for the old status quo, and to post a workaround for others in my situation:

# Workaround.  Add this to your .vimrc or whatever.
au BufRead,BufNewFile *.pp setlocal ft=puppet
au FileType puppet setlocal iskeyword+=:

I generate my tags files with Exuberant Ctags. The tags file is loaded into Vim in the usual manner. I am still able to jump to a tag by manually entering its name as follows:

:tag name

This would lead me to believe the tags file itself is all hunky-dory.

Of course, people seldom manually enter tag names. It is much more convenient to position the cursor over a keyword and strike CTRL-]. This used to work, but has not worked for a while.

I shall walk through an example using a real manifest with names redacted.

class contosso::flibproxy {
  $instances = lookup('flibproxy_instances', Array, first, [])

  contosso::flibproxy::instance { $instances: }
}

Previously, with older revisions of vim-puppet, I could position my cursor anywhere along contosso::flibproxy::instance (let's say the first c) and strike CTRL-] to be teleported to the definition of that type. Today, Vim refuses that request with a whimsical error message:

E257: cstag: tag not found

Because that error message was too general to help isolate the problem, I tried some other things at random, including g CTRL-]. This elicited a more useful error message:

E426: tag not found: contosso

Where Vim previously recognised the fully-qualified resource name contosso::flibproxy::instance, it seems Vim is now only recognising the substrings separated by colons (contosso, flibproxy, and instance).

As noted earlier, I see this keyword change was made with intent, so I understand if you are unable to accommodate conflicting requirements and choose to close this report as WAI.

lelutin commented 2 years ago

@saj thanks for tracking down what had changed and caused a behaviour to have changed!

As mentioned in #137 using : in iskeyword leads to some other confusion where this character is used in a way that's not part of an actual keyword (e.g. in the example you gave above, the variable name used as the resource keyword $instances: would all be taken in as a keyword even though in this case the colon character is used as a delimiter between resource name and parameters).

But maybe it would be a good idea to document the change of behaviour and the way to bring the old behaviour back in the readme?

what would be even greater would be to bring back proper recognition of keywords for object names containing ::, but I currently don't know how to do this without the side-effects mentioned above.

would you be willing to prepare a pull request for adding documentation in the README file on how to bring back keyword recognition?

saj commented 2 years ago

what would be even greater would be to bring back proper recognition of keywords for object names containing ::, but I currently don't know how to do this without the side-effects mentioned above.

Indeed. Unfortunately, I don't think this is (currently) possible in vim.

CTRL-] is implemented using nv_ident(). The same function is used to implement * and #.

if (ptr == NULL && (n = find_ident_under_cursor(&ptr,
        (cmdchar == '*' || cmdchar == '#')
             ? FIND_IDENT|FIND_STRING : FIND_IDENT)) == 0)

find_ident_under_cursor():

/*
 * Find the identifier under or to the right of the cursor.
 * "find_type" can have one of three values:
 * FIND_IDENT:   find an identifier (keyword)
 * FIND_STRING:  find any non-white text
 * FIND_IDENT + FIND_STRING: find any non-white text, identifier preferred.
 * FIND_EVAL:    find text useful for C program debugging
 * ...
 */
    int
find_ident_under_cursor(char_u **text, int find_type)
{

This function essentially boils down to a bunch of calls to mb_get_class() with the goal of searching for transitions between character classes. Digging a little further shows the classes are implemented using a simple bitmap that is initialised from the user's iskeyword option.

People who write C++ must run into a similar problem. I wonder what they do. Perhaps a better, more generalised, solution would be to generate tags files with scope metadata. Instead of this...

contosso::flibproxy::instance       local-modules/contosso/manifests/flibproxy/instance.pp   /^define contosso::flibproxy::instance() {$/;"   d

...perhaps I should be generating tags that look like this:

instance       ...       module=contosso::flibproxy

I figure that would be compatible with an isk option value sans :. However:

Suffice to say that I am now more confused than when I began looking into this problem, and am unsure whether I should be leading others down my path. :-) I trust this discussion will appear in a targeted web search; anyone arriving here in the future can make up their own mind. Perhaps folks who are looking for accurate jump-to-definition would be better served by a language server. All I can state with any modicum of certainty is that what I have now -- vim-puppet and isk+=: -- is at least lightweight and works well enough for me.

Thanks for your continued maintenance!