preservim / vim-textobj-quote

Use ‘curly’ quote characters in Vim
Other
123 stars 6 forks source link

Add `bar`: help autocommands use custom commands #22

Closed telemachus closed 4 years ago

telemachus commented 4 years ago

Without -bar, users must manually call the underlying functions for Educate, NoEducate, and ToggleEducate in autocommands.

That is, without -bar the following won't work:

autocmd vim_config InsertCharPre *.md if v:char == '`' | ToggleEducate | endif

You have to do this instead:

autocmd vim_config InsertCharPre *.md if v:char == '`' | call textobj#quote#educate#toggleMappings() | endif

Thanks for considering this.

alerque commented 4 years ago

Hmm, interesting! Definitely looks like an improvement to be, but since I am unfamiliar with the -bar option...

Are there any downsides to this? Does it cause parsing overhead even when not in use? Will it obsolete other configurations because the old syntax won't work? What if any are the possible caveats here?

telemachus commented 4 years ago

@alerque I am not myself an expert on this, but my understanding from #vim on Freenode and from reading the manual is that there is only one potential downside, but it doesn't apply in this case. You would not want use -bar if the command you were creating was meant to consume | characters. For example, if you're writing a command that expects regexes, and those regexes can contain alternations (bizz|buzz), you should not add -bar. But any other custom command should use -bar for just the purpose I mention in the commit: to allow other users to put the new command into autocommands more easily.

tl;dr - it helps in cases like the example I gave, and I don't believe that there is any downside. Documentation:

alerque commented 4 years ago

Thanks for that research. Looks great to me, thanks for taking the time to contribute.

Is there anywhere in the documentation that should be updated to reflect the more straightforward syntax?

telemachus commented 4 years ago

Is there anywhere in the documentation that should be updated to reflect the more straightforward syntax?

Good thought. Yes, I suppose a brief mention would be good. I would put it right after this section:

You can change educating behavior with the following commands:

  • Educate
  • NoEducate
  • ToggleEducate

Maybe a single sentence with an example?

These three commands can also be used in autocommands:

autocmd InsertCharPre *.md if v:char == '`' | ToggleEducate | endif

Since the documentation doesn't currently mention using the underlying functions in autocommands, there's nothing that needs to be removed or changed.

Btw, I just noticed for the first time that the plugin doesn't come with documentation for vim itself. Would you be open to a PR for internal documentation? (I would probably just take big chunks from the README, though I'm open to other suggestions.)

Thanks.

alerque commented 4 years ago

I just noticed for the first time that the plugin doesn't come with documentation for vim itself. Would you be open to a PR for internal documentation?

Yes absolutely!

You might even minimize the wording in the README to the very initial setup and basic configuration, and point people to how to open the :help docs for more advanced configuration options. We could list generally what they are in the README for the sake of people evaluating whether or not to install the plugin, but nitty-gritty-details are better suited to the internal help docs.

telemachus commented 4 years ago

@alerque Okay. I will try to work on that later in the week or this weekend. One other question: did you want a separate PR for the addition to the readme in the meantime, will you add that yourself, or would you rather skip it and focus on shifting most of the readme content to the docs?

alerque commented 4 years ago

I figured as long as this change didn't mean anything different for existing users and no existing documentation touched on the possibility — and that there might be a docs overhaul coming up — there was no reason to hold up this PR waiting for it.

I don't care if there is a separate PR, but I'd be inclined to just have a separate commit somewhere in the docs overhaul PR as you work on it.

No pressure by the way, we'd love proper docs but understand it won't be a high priority project. I'll open a separate issue to track progress so it doesn't get forgotten entirely before a PR exists.