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

coallesce mayset queries #192

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mralusw
Copy link

@mralusw mralusw commented Mar 20, 2024

Well, you were right in #191 that anything other then verb setg all q? moves the cursor. That was not readily apparent from the repo's history, and I still can't find the explanation. I've tried several approaches with no luck.

The approach in this PR is to query all settings of interest at once in the beginning. Finding the appropriate query + answer for each checked setting, then, requires just a little tweaking of your initial "home" regex. A downside is that any future queries will need to be registered in s:mayset_checks before usage.

It may not matter much on modern machines, but the previous version is a little slow on old boxes and on "semi"-embedded — at least as a percentage of a modest vimrc's startup time. Make of it what you will.

I've left some debug checks commented out, as they might come in handy.

@tpope
Copy link
Owner

tpope commented Mar 20, 2024

A downside is that any future queries will need to be registered in s:mayset_checks before usage.

Let's fix this. Instead of s:MaySet(), let's have a s:DefaultSet() that records the option and value.

let s:defaults = {}
function! s:DefaultSet(option, value) abort
  let s:defaults[a:option] = a:value
endfunction

At the bottom of the plugin, we can set each option after we check for it.

Please limit to Vim 7.4 compatible syntax: No lambdas, no #{}.

@mralusw mralusw force-pushed the feat-coallesce-mayset-queries branch from 9d3528d to 5f31747 Compare March 21, 2024 04:59
@mralusw
Copy link
Author

mralusw commented Mar 21, 2024

Great idea — I've just force-pushed an implementation. It's still faster than before and it also simplifies the configuration part.

At the bottom of the plugin, we can set each option after we check for it.

… though I'm not sure if :set order matters for some of these options. I've tried to minimize changes. In particular, all MaySet() defaults are executed in one fell swoop just before the syntax / ftplugin / mappings, and in the same relative order, just as they would occur previously (instead of at the very end as you seem to suggest). However, non-MaySet() defaults are all executed before the MaySet() ones.

@mralusw mralusw force-pushed the feat-coallesce-mayset-queries branch from 5f31747 to f039d07 Compare March 21, 2024 12:43
@mralusw mralusw force-pushed the feat-coallesce-mayset-queries branch from 6550d9c to 13bd446 Compare March 24, 2024 16:56
@mralusw
Copy link
Author

mralusw commented Mar 24, 2024

I've cleaned up the commits and added code to support un-queried (non-Mayset()) options via the same interface. I don't really see the point of forcing all :set's to go through the same interface, TBH, so the last two commits are probably overkill.

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.

2 participants