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.

Sign up for GitHub

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

Jump to bottom

Allow toggling numeric configs #4336

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Allow toggling numeric configs #4336

wants to merge 5 commits into from

Conversation

gahr
Copy link
Member

@gahr gahr commented Jun 27, 2024 •
edited
Loading

This allows the toggle command to work with numeric configs, by switching their value from whatever they are to 0 and back.

This can be useful for temporarily turning "off" features like pager_index_lines or wrap.

Fixes #4333

@gahr gahr added type:discuss Your views/opinions are requested type:enhancement Feature Request labels Jun 27, 2024
@gahr gahr requested review from roccoblues and flatcap June 27, 2024 11:36
@gahr gahr self-assigned this Jun 27, 2024
@gahr gahr requested a review from a team as a code owner June 27, 2024 11:36
@gahr gahr force-pushed the devel/issue-4333 branch 4 times, most recently from 265e274 to 462c7ca Compare June 27, 2024 12:55
roccoblues
roccoblues reviewed Jun 27, 2024
View reviewed changes
Copy link
Member

@roccoblues roccoblues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼 I like it. toggle also works for me.

I just stumbled upon the no/inv prefix error message. Maybe you can shed some light on it?

config/number.c Show resolved Hide resolved
parse/set.c Show resolved Hide resolved
gahr added 4 commits June 28, 2024 08:31
@gahr
Allow toggling numeric configs
a8d5d1a 
This allows the `toggle` command to work with numeric configs, by
switching their value from whatever they are to 0 and back.

This can be useful for temporarily turning "off" features like `pager_index_lines` or `wrap`.

Fixes #4333
@gahr
Don't trigger UB
31be6a9
@gahr
Make sure setting a variable clears the toggled state
51b40eb
@gahr
Unsetting a numeric value sets it to 0
7ce15a3
@gahr
Copy link
Member Author

gahr commented Jun 28, 2024

👍🏼 I like it. toggle also works for me.

I just stumbled upon the no/inv prefix error message. Maybe you can shed some light on it?

  • inv: equivalent to toggle
  • no: I made it work like set <foo>=0

@gahr
Copy link
Member Author

gahr commented Jun 29, 2024

Thanks @roccoblues . Let's wait and see what @flatcap thinks. If I recall correctly, we have discussed set nowrap at some poiny and he had arguments against it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roccoblues roccoblues roccoblues approved these changes

@flatcap flatcap Awaiting requested review from flatcap

Assignees

@gahr gahr

Labels
type:discuss Your views/opinions are requested type:enhancement Feature Request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Toggle pager_index_lines
2 participants
@gahr @roccoblues