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

List of mwes that panic #43

Closed
ammkrn opened this issue Jun 2, 2021 · 8 comments
Closed

List of mwes that panic #43

ammkrn opened this issue Jun 2, 2021 · 8 comments
Labels
C-bug Category: This is a bug

Comments

@ammkrn
Copy link
Contributor

ammkrn commented Jun 2, 2021

These are minimal working examples of inputs after hx that cause a panic, tested on ubuntu and macOS.

j, b

j, w

j, e

j, f, f

d, f, f

d, f, n

i, a, esc, b, w

i, a, esc, b, e

and someone else brought up d, d.

I tried making the calls to slice.char(_) safe in the varioushelix-core::movement functions, but that caused off by ones in coords_at_pos and something in the compositor, I'm not sure if it's because of bad indexing elsewhere, or if my "fixes" aren't obeying rules about the anchor/head points for Range. Others have also noted that it seems like you can move down one more line than you would be able to in IE kakoune, but that doesn't seem to explain the d, d or b cases.

Just poking around, there seems to be a lot of interaction with ropey via partial functions which may not be ideal in the long run. It's a bummer that ropey doesn't seem to offer total versions of the indexing functions.

Cool project by the way, I wish you the best of luck.

@archseer
Copy link
Member

archseer commented Jun 2, 2021

I think I addressed some of these in the latest push, it's mostly of by one errors or missing bounds checks -- I agree that having a safe indexing wrapper would be useful as well.

@ammkrn
Copy link
Contributor Author

ammkrn commented Jun 2, 2021

Thanks. It looks like the remaining ones are d, d, i, a, esc, b, w and i, a, esc, b, e. If the ropey maintainers are interested in having an api extension that doesn't panic, I'll see about adding one.

@archseer
Copy link
Member

archseer commented Jun 2, 2021

After f4560cb just dd left :)

@ammkrn
Copy link
Contributor Author

ammkrn commented Jun 2, 2021

Right on. The ropey maintainer(s) agreed to accept the api extension, I'm waiting for them to review the initial implementation. I'll @ mention you if/when it gets pulled into master.

@heliostatic
Copy link
Contributor

Fantastic! Just came to report this (with backtrace) but delighted to see it will be fixed upstream.

@cessen
Copy link
Contributor

cessen commented Jun 3, 2021

Re: non-panicking version of Ropey's functions

@ammkrn and I have been discussing this in cessen/ropey#39. @archseer: if you could join the discussion over there, as the primary author of Helix, I'd really appreciate it!

@archseer archseer added the C-bug Category: This is a bug label Jun 3, 2021
@jbaa
Copy link
Contributor

jbaa commented Jun 3, 2021

I believe the issue with dd was fixed with #90.

@ammkrn
Copy link
Contributor Author

ammkrn commented Jun 3, 2021

@jbaa Seems like it to me. Since the ropey api discussion is happening there, I'll go ahead and close this. Thanks!

@ammkrn ammkrn closed this as completed Jun 3, 2021
makemeunsee pushed a commit to makemeunsee/helix that referenced this issue Oct 14, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml
makemeunsee pushed a commit to makemeunsee/helix that referenced this issue Oct 14, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml & tokio features
makemeunsee pushed a commit to makemeunsee/helix that referenced this issue Oct 26, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml
makemeunsee pushed a commit to makemeunsee/helix that referenced this issue Oct 26, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml & tokio features
makemeunsee added a commit to makemeunsee/helix that referenced this issue Oct 26, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml
makemeunsee added a commit to makemeunsee/helix that referenced this issue Oct 26, 2023
broken `cargo build`, fix by uncommenting
lines helix-editor#43-46 of helix-tui/Cargo.toml & tokio features
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

5 participants