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

Enhance :toggle to support cycling numbers #7877

Conversation

alevinval
Copy link
Contributor

@alevinval alevinval commented Aug 8, 2023

Update toggle implementation to accept toggling numbers. Thanks to #4411 (comment) where it became clear that adding support for it is quite trivial.

anyhow::bail!("Configuration {key} does not support toggle yet")
}
};

let status = format!("'{key}' is now set to {value}");
let config = serde_json::from_value(config)
.map_err(|_| anyhow::anyhow!("Could not parse field: `{:?}`", &args))?;
.map_err(|err| anyhow::anyhow!("Error parsing `{:?}`, reason: {:?}", &args, err))?;
Copy link
Contributor Author

@alevinval alevinval Aug 8, 2023

Choose a reason for hiding this comment

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

I felt like updating the error message so its clear to the user why its failing, see how it will look like in case someone puts decimals or negative integers:

Screenshot 2023-08-08 at 18 17 37

@alevinval alevinval force-pushed the issue-4411-support-toggle-numbers branch from c5943c4 to 7ffa1dd Compare August 8, 2023 16:17
@the-mikedavis the-mikedavis changed the title enhance toggle to support cycling numbers Enhance :toggle to support cycling numbers Aug 9, 2023
@the-mikedavis the-mikedavis merged commit 48eb0d4 into helix-editor:master Aug 9, 2023
6 checks passed
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
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.

3 participants