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

Do not crash when run goto command without line number #1160

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

wingyplus
Copy link
Contributor

@wingyplus wingyplus commented Nov 24, 2021

Report an error when running goto command without entering a
line number.

Fixes #1159

@wingyplus wingyplus changed the title Do not crash when run goto command without line number Do not crash when run goto command without a line number Nov 24, 2021
@wingyplus wingyplus changed the title Do not crash when run goto command without a line number Do not crash when run goto command without line number Nov 24, 2021
@@ -2485,6 +2485,10 @@ mod cmd {
args: &[&str],
_event: PromptEvent,
) -> anyhow::Result<()> {
if args.len() == 0 {
return Ok(());
Copy link
Contributor

Choose a reason for hiding this comment

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

We should return an error here, rather than failing silently.

Suggested change
return Ok(());
bail!("Line number required");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that would be better than silencing the false path. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Omnikar Fixed. :)

Report an error when running goto command without entering a
line number.

Fixes helix-editor#1159
helix-term/src/commands.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@s1ck s1ck left a comment

Choose a reason for hiding this comment

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

Since I introduced the feature recently (it was merged today) I felt like giving a review :)

Thanks @wingyplus for catching it. I'm not sure how this could be caught by testing 🤔
Anyways, the fix is solid and solves the problem 👍

@archseer archseer merged commit e8f800a into helix-editor:master Nov 25, 2021
@wingyplus wingyplus deleted the goto-not-crash branch November 25, 2021 02:03
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.

:goto should not crash if it doesn't input the line number?
4 participants