5 ms·

I gave Helix a shot, and I immediately ran into a wall due to an undocumented limitation where in a language configuration, indentation can only be up to 8 spac

I gave Helix a shot, and I immediately ran into a wall due to an undocumented limitation where in a language configuration, indentation can only be up to 8 spaces. (I'm working on an Assembly project with an indentation of 12 spaces to make more room for labels.) I checked the source code to figure out what the heck was going on, and I found this: https://github.com/helix-editor/helix/blob/207829eefee51095d1c7b424cf11cab694fbf52b/helix-core/src/indent.rs#L39-L59 https://github.com/helix-editor/helix/blob/207829eefee51095d... Pretty often, my projects have weirdly specific needs like this. I think (Neo)Vim's greatest strength is that it always absolutely has some way of covering a weirdly specific need, whether a one-off command or a project configuration. But that seems to be against Helix's philosophy. If Helix can't be the editor, then I can't justify taking the time to learn it. It makes me sad, though, because I really like what it's going for.

Sirenos 3y ago
That looks very hacky. Is there a reason for this limitation that you've found makes sense?
- otikik 3y ago
  It seems relatively easy to send a PR to allow supporting any number of spaces (or at least increase the upper limit to 16).
  - messe 3y ago
    > allow supporting any number of spaces As long as nothing is depending on 'static lifetime further down the line. (EDIT: ignore and see below) > (or at least increase the upper limit to 16) That should be doable for sure. Even something like (I'm not a rust developer, just have a passing familiarity, so my syntax may be off): IndentStyle::Spaces(n) if 0 < n && n <= 16 => &" "[0..n as usize], I think that would condense it all down into one line, rather than having special cases for all of 1 through 16. EDIT: after toying around in the rust playground, I think the following will support any indent level (within the bounds of the u8 type). At the top level: static INDENTS: [u8: 256] = [' '; 256]; And then in the as_str function: IndentStyle::Spaces(n) => std::str::from_utf8(&INDENTS[0..n as usize]).unwrap()
    - ziml77 3y ago
      The [0..n] slicing is what I was surprised to not see being done. As for what you're suggesting, using from_utf8 adds a overhead to check that you're giving it valid UTF-8 (this does not get optimized away). Dipping into unsafe can help here: const INDENTS: [u8; 256] = [b' '; 256]; // ... IndentStyle::Spaces(n) => unsafe { std::str::from_utf8_unchecked(&INDENTS[0..n as usize]) }, But I would consider having INDENTS be a &str to get the cheap slicing without needing unsafe. Since putting in a string literal of 256 spaces is nasty I would use the const_format crate to generate the constant: const INDENTS: &str = str_repeat!(" ", 256);
      - messe 3y ago
        I'm not sure I could justify bringing in a package for a one-liner, just to avoid an "unsafe" which could be easily documented in a comment as actually safe.
      - heliostatic 3y ago
        Closing the loop, this is now implemented and on main, thanks to msdrigg: https://github.com/helix-editor/helix/pull/7429 https://github.com/helix-editor/helix/pull/7429
        messe 3y ago
        Nice!
        otikik 3y ago
        This makes me happy, thanks for sharing
- sph 3y ago
  That doesn't look hacky, it looks written by someone with 3 months of programming experience. I would personally avoid relying on an editor written by programming newbies.
  - xojoc 3y ago
    Maybe it's done like that for performance reasons?
    - messe 3y ago
      I'm struggling to see how that function could be a bottleneck, and if it were, memoization would easily get rid of it without the ridiculous limitation.
      - bobbyskelton41 3y ago
        It could be better for sure, but that function has been there for almost as long as the project has been open-source if I recall correctly. It probably hasn't been touched since then because it wasn't a high priority.
  - mat_epice 3y ago
    Something something logical fallacy [1]. [1] https://en.m.wikipedia.org/wiki/Ad_hominem https://en.m.wikipedia.org/wiki/Ad_hominem
    - unshavedyak 3y ago
      I always find it interesting how hostile some (many?) peoples take on source code can be. Rather than give benefit of doubt, they attack. But even if it is some stupid code, some late-night, tired-brain, foggy-flu-delirium .. so what? I get it, don't run a project if you see some bad code and question the quality of the project. Quality that very well could, and likely will, impact you. From bugs to performance. Don't run it, of course. But to attack it, seemingly with such personal zeal.. can people not improve? I dunno, i guess i just don't feel the software we write on average is actually good. We make compromises and mistakes constantly. Our house is the purist of glass and many of us are just chucking stones like there's no tomorrow. Lets take a step back. Breathe. Point out flaws by all means, but maybe reduce the often apparent revelry in someone else's mistake. ... sorry for the high horse rant. I'm nursing coffee and i've just seen it a lot with Lemmy/Kbin recently. Also this isn't pointed at anyone here directly.. just a pattern i feel i frequently see.
- msdrigg 3y ago
  Its doing that to get a static str (no allocations). I assume it is for performance reasons, but I can’t speak to whether it’s actually a significant impact.
  - unshavedyak 3y ago
    The real flaw here in my mind is the lack of comments :D If this was a carefully observed decision, one that is prone to looking odd, it deserves an explanation for future selves to not chase the rabbit of understanding.
- tsukikage 3y ago
  A sane implementation would be for the fallback to construct a string containing the desired number of spaces on the fly and memoize. Unfortunately, this is written in Rust, and the language makes it super awkward to implement caching patterns transparently as they involve passing around references to mutable state.
  - bfrog 3y ago
    There’s a dozen ways to solve this… from arc to ref cell, the language provides all sorts of methods to solve this.
eviks 3y ago
> always absolutely has some way of covering a weirdly specific need what's the (Neo)Vim way of covering the need for elastic tabstops, multicursor editing, differentiating left vs right modifiers and tap vs hold regular keys?
- dansalvato 3y ago
  Multicursor editing, left/right modifiers, and tap/hold keys aren't "needs", they're methods/tools used to accomplish needs. The need is to make a specific edit on a file. I respect those features, and I respect Helix as a great forward-thinking editor. I will just have to revisit it in the future once it's more configurable.
survirtual 3y ago
The great thing about Helix is that it is super easy to build locally. Make changes to the number of spaces allowed and it should work as expected, unless there is another reason they have such a limit.

Sirenos 3y ago

That looks very hacky. Is there a reason for this limitation that you've found makes sense?

otikik 3y ago
It seems relatively easy to send a PR to allow supporting any number of spaces (or at least increase the upper limit to 16).
- messe 3y ago
  > allow supporting any number of spaces As long as nothing is depending on 'static lifetime further down the line. (EDIT: ignore and see below) > (or at least increase the upper limit to 16) That should be doable for sure. Even something like (I'm not a rust developer, just have a passing familiarity, so my syntax may be off): IndentStyle::Spaces(n) if 0 < n && n <= 16 => &" "[0..n as usize], I think that would condense it all down into one line, rather than having special cases for all of 1 through 16. EDIT: after toying around in the rust playground, I think the following will support any indent level (within the bounds of the u8 type). At the top level: static INDENTS: [u8: 256] = [' '; 256]; And then in the as_str function: IndentStyle::Spaces(n) => std::str::from_utf8(&INDENTS[0..n as usize]).unwrap()
  - ziml77 3y ago
    The [0..n] slicing is what I was surprised to not see being done. As for what you're suggesting, using from_utf8 adds a overhead to check that you're giving it valid UTF-8 (this does not get optimized away). Dipping into unsafe can help here: const INDENTS: [u8; 256] = [b' '; 256]; // ... IndentStyle::Spaces(n) => unsafe { std::str::from_utf8_unchecked(&INDENTS[0..n as usize]) }, But I would consider having INDENTS be a &str to get the cheap slicing without needing unsafe. Since putting in a string literal of 256 spaces is nasty I would use the const_format crate to generate the constant: const INDENTS: &str = str_repeat!(" ", 256);
    - messe 3y ago
      I'm not sure I could justify bringing in a package for a one-liner, just to avoid an "unsafe" which could be easily documented in a comment as actually safe.
    - heliostatic 3y ago
      Closing the loop, this is now implemented and on main, thanks to msdrigg: https://github.com/helix-editor/helix/pull/7429 https://github.com/helix-editor/helix/pull/7429
      - messe 3y ago
        Nice!
      - otikik 3y ago
        This makes me happy, thanks for sharing
sph 3y ago
That doesn't look hacky, it looks written by someone with 3 months of programming experience. I would personally avoid relying on an editor written by programming newbies.
- xojoc 3y ago
  Maybe it's done like that for performance reasons?
  - messe 3y ago
    I'm struggling to see how that function could be a bottleneck, and if it were, memoization would easily get rid of it without the ridiculous limitation.
    - bobbyskelton41 3y ago
      It could be better for sure, but that function has been there for almost as long as the project has been open-source if I recall correctly. It probably hasn't been touched since then because it wasn't a high priority.
- mat_epice 3y ago
  Something something logical fallacy [1]. [1] https://en.m.wikipedia.org/wiki/Ad_hominem https://en.m.wikipedia.org/wiki/Ad_hominem
  - unshavedyak 3y ago
    I always find it interesting how hostile some (many?) peoples take on source code can be. Rather than give benefit of doubt, they attack. But even if it is some stupid code, some late-night, tired-brain, foggy-flu-delirium .. so what? I get it, don't run a project if you see some bad code and question the quality of the project. Quality that very well could, and likely will, impact you. From bugs to performance. Don't run it, of course. But to attack it, seemingly with such personal zeal.. can people not improve? I dunno, i guess i just don't feel the software we write on average is actually good. We make compromises and mistakes constantly. Our house is the purist of glass and many of us are just chucking stones like there's no tomorrow. Lets take a step back. Breathe. Point out flaws by all means, but maybe reduce the often apparent revelry in someone else's mistake. ... sorry for the high horse rant. I'm nursing coffee and i've just seen it a lot with Lemmy/Kbin recently. Also this isn't pointed at anyone here directly.. just a pattern i feel i frequently see.
msdrigg 3y ago
Its doing that to get a static str (no allocations). I assume it is for performance reasons, but I can’t speak to whether it’s actually a significant impact.
- unshavedyak 3y ago
  The real flaw here in my mind is the lack of comments :D If this was a carefully observed decision, one that is prone to looking odd, it deserves an explanation for future selves to not chase the rabbit of understanding.
tsukikage 3y ago
A sane implementation would be for the fallback to construct a string containing the desired number of spaces on the fly and memoize. Unfortunately, this is written in Rust, and the language makes it super awkward to implement caching patterns transparently as they involve passing around references to mutable state.
- bfrog 3y ago
  There’s a dozen ways to solve this… from arc to ref cell, the language provides all sorts of methods to solve this.

eviks 3y ago

> always absolutely has some way of covering a weirdly specific need what's the (Neo)Vim way of covering the need for elastic tabstops, multicursor editing, differentiating left vs right modifiers and tap vs hold regular keys?

dansalvato 3y ago
Multicursor editing, left/right modifiers, and tap/hold keys aren't "needs", they're methods/tools used to accomplish needs. The need is to make a specific edit on a file. I respect those features, and I respect Helix as a great forward-thinking editor. I will just have to revisit it in the future once it's more configurable.

survirtual 3y ago

The great thing about Helix is that it is super easy to build locally. Make changes to the number of spaces allowed and it should work as expected, unless there is another reason they have such a limit.