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

Prevent a panic when uncommenting a line with only a comment token #5933

Merged
merged 1 commit into from
Feb 12, 2023

Conversation

trink
Copy link
Contributor

@trink trink commented Feb 12, 2023

Open a new document test.rs and type the following: di//<esc><C-c>

The margin calculation pushes the range out of bounds for the comment marker when there are no characters (newline) after it.

thread 'main' panicked at 'called Result::unwrap() on an Err value: Char range out of bounds: char range 0..3,
Rope/RopeSlice char length 2', ropey-1.6.0/src/rope.rs:546:37

The debug build catches the error in the transaction: thread 'main' panicked at 'attempt to subtract with overflow',
helix-core/src/transaction.rs:503:26

Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

Nice, catch! A minor cosmetic nit, otherwise this LGTM

helix-core/src/comment.rs Outdated Show resolved Hide resolved
@pascalkuthe pascalkuthe added C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-core Area: Helix core improvements labels Feb 12, 2023
@trink trink force-pushed the panic_on_uncomment branch from f146feb to 6a2af5d Compare February 12, 2023 15:00
@pascalkuthe pascalkuthe added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 12, 2023
pascalkuthe
pascalkuthe previously approved these changes Feb 12, 2023
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

if matches!(line_slice.get_char(pos + token_len), Some(c) if c != ' ') {
margin = 0;
if !matches!(line_slice.get_char(pos + token_len), Some(c) if c == ' ') {
margin = 0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
margin = 0
margin = 0;

looks like this semicolon got chopped off on accident

Open a new document `test.rs` and type the following:
`di//<esc><C-c>`

The margin calculation pushes the range out of bounds for the comment
marker when there are no characters (newline) after it.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
Char range out of bounds: char range 0..3,
Rope/RopeSlice char length 2', ropey-1.6.0/src/rope.rs:546:37

The debug build catches the error in the transaction: thread 'main'
panicked at 'attempt to subtract with overflow',
helix-core/src/transaction.rs:503:26
@the-mikedavis the-mikedavis merged commit ef221ab into helix-editor:master Feb 12, 2023
@trink trink deleted the panic_on_uncomment branch March 2, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants