Hacker News Reader

Comment by dansalvato | Hacker News Reader

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/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.

View on HN

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

It seems relatively easy to send a PR to allow supporting any number of spaces (or at least increase the upper limit to 16).

> 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()

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);

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.

Closing the loop, this is now implemented and on main, thanks to msdrigg: https://github.com/helix-editor/helix/pull/7429

Nice!

This makes me happy, thanks for sharing

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.

Maybe it's done like that for performance reasons?

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.

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.

Something something logical fallacy [1].

[1] https://en.m.wikipedia.org/wiki/Ad_hominem

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.

What? No. An ad hominem is "I think your argument is wrong because your face is stupid". "This is bad software because it looks like it's made by amateurs" isn't attacking their character, it's attacking their ability to write good software, which is highly relevant.

"The code is bad because it was written by a novice dev" sounds like an ad hominem attack to me (as opposed to "the code is bad because of X attribute of the code itself").

Ad hominem attack aside... it was chosen because they didn't see a realistic need for more than 8 spaces. Nobody has openly complained in the past 2 years Helix has been around until now, so they were at least mostly right.

It looks to me like it was written by someone quite intentionally to avoid allocations in the common case, and they punted on handling less common cases to later.

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.

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.

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.

There’s a dozen ways to solve this… from arc to ref cell, the language provides all sorts of methods to solve this.

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.

> 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?

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.

Reply on news.ycombinator.com