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

fix: bug where restoring value when leaving minimalist mode #138

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ilan-schemoul
Copy link

Problem

This fixes a bug where leaving minimalist mode crashes the plugin. If you have for example "set numberwidth=1" the current code will interpret 1 as a boolean value. So when we leave minimalist mode the plugin naively tries to do "set numberwidth=true" which leads to a crash. The reason the current code tries to intepret 1 as boolean is that sometimes it does represent a boolean. But always intepreting numbers as boolean is naive and leads to crashes.

Solution

We save numbers as numbers (instead of saving them as booleans if the number = 1).
Then let's say we try to restore the value "ruler" and the original value is "1" (as it is how vim represents a boolean) then we will try "set ruler=1". It will produce a bug which will be detected by pcall. So now we enter the if block and we try to do "set ruler" which works.

Observation 1

(type(opt) == "number" and (opt == 1 and true or false) or opt) nested ternary are hard to read and confusing. Also opt == 1 and true or false can be replaced by opt == 1
Instead of the ternary statement we can do:

if type(opt) == "number" then
  original_opts[user_opt] = opt == 1
else
 original_opts[user_opt] == op
end

is much much clearer. I have deleted this part of the code as I don't need it anyway but I would advise to prioritize clean, simple code (with a bit more comments) as opposed to unclear code with nested ternary statements.

When leaving minimalist mode we restore all values. Sometimes a value is "1"
or "0" (to represent booleans) but cannot be restored with "set $OPTION=1" we
must then use "set $OPTION" or "set no$OPTION".
E.g: ruler is enabled (set ruler). When we insert minimalist mode ruler
original value is saved with the value "1". When we restore original options
we use "set ruler" instead of "set ruler=1" (which will crash).
@ilan-schemoul
Copy link
Author

@pocco81 can you please take a look ?

@ilan-schemoul
Copy link
Author

@pocco81 hey please I'd really love that you'd take a look

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.

1 participant