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

Weird behavior with high number modifiers #10922

Closed
MyApaulogies opened this issue Jun 10, 2024 · 11 comments · Fixed by #10930
Closed

Weird behavior with high number modifiers #10922

MyApaulogies opened this issue Jun 10, 2024 · 11 comments · Fixed by #10930
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@MyApaulogies
Copy link

Summary

After typing 20+ digits in normal mode, the number display starts changing erratically with each new digit. Then at around the 60th digit, the number resets or freezes.

Reproduction Steps

I tried this:

  1. hx
  2. 1 then spam 0 (disappears)
  3. spam 1, j (number freezes, j works)
  4. gg, spam 2, j (number freezes, j doesn't work)

I expected the number to cap at a reasonable amount, such as 15 digits, the width of the number display.

Instead, the number keeps going and has unpredictable behavior.

Helix log

No response

Platform

Linux

Terminal Emulator

Alacritty

Installation Method

dnf

Helix Version

helix 24.3 (2cadec0)

@MyApaulogies MyApaulogies added the C-bug Category: This is a bug label Jun 10, 2024
@RoloEdits
Copy link
Contributor

Can you take a screencap of it happening? I tried the steps, but I got the expected appending, and the expected view that follows the cursor beyond the normal screen size, and the digits showed what I expected.

Are you using soft-wrap?

@MyApaulogies
Copy link
Author

helix.numbers.webm

Yes I'm using soft-wrap. I'm curious, how that would affect it?

@MyApaulogies
Copy link
Author

Also, I didn't have any relevant lines in my log before, but now this happened in the last few minutes:

2024-06-10T20:52:22.003 helix_core::syntax [ERROR] TS parser failed, disabling TS for the current buffer: Err(Cancelled)

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jun 11, 2024
@RoloEdits
Copy link
Contributor

Ah, I misunderstood the reproduction steps. I thought you were typing in the normal buffer. When I saw gg I just assumed so. I skimmed passed the in normal mode part.

The last part though, gg, spam 2, j (number freezes, j doesn't work), j works for me.

@MyApaulogies
Copy link
Author

Ah yeah sorry, wasn't quite sure how to refer to that number.

@RoloEdits
Copy link
Contributor

RoloEdits commented Jun 11, 2024

Checked in debug mode it panicked with: attempt to multiply with overflow.

I can probably get a pr for this with a fix, but what behavior should be desired? If we cap it to digits then it could go from 1111111111111111 and then suddenly switch to 999999999999999. If we go by actual length, then we would need to turn the number to a string to check it, and then if it goes over, should it drop the left most digit, as the displayed value grows right, or should it just return the previous last good value(which is just functionally dropping the right most digit)?

@RoloEdits
Copy link
Contributor

This is it dropping the oldest(left most) digit:
overflow_drop_oldest_helix

This is it capped:
overflow_capped_helix

And this is it returning the previous good value(I keep pressing but as it keeps returning the same value there is no change in displayed number):
overflow_return_old_value

@MyApaulogies
Copy link
Author

Oh, very cool! My vote is on dropping digits, just because it has a clear visual indication of where your inputs are going in case you think you're in insert mode. (because who's really going to need precise numbers in the trillions?)

@RoloEdits
Copy link
Contributor

On the other hand, the standard use case for a count would never encounter an overflow. So not sure its worth adding a branch and an allocation each use to solve something that's likely to never be encountered when using the feature. Before submitting the pr, ill wait for feedback from the core maintainers to see what they think.

@RoloEdits
Copy link
Contributor

@kirawi or @the-mikedavis some direction would be helpful, if possible.

@the-mikedavis
Copy link
Member

I think it's reasonable to cap to a high number like 100,000,000. If a keypress of a number would make the count exceed that value we can just ignore the keypress. This situation shouldn't come up often so I'd like to keep it as simple as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@RoloEdits @the-mikedavis @MyApaulogies @kirawi and others