add basic modeline support

[doy]

fixes #729

currently only supports setting language, indent style, and line endings, but maybe this is enough?

[pascalkuthe]

This looks pretty good, thanks for taking this on!

[pascalkuthe]

Also one more thing: I think the modeline.rs file can/should be moved to helix-core

[the-mikedavis]

To me this feels like plugin territory rather than something that should be built into the core. I believe only each editor supports their own modeline natively: vim variants are the only ones to support vim modelines, only emacs supports emacs modelines natively, vscode doesn't support any modelines natively AFAIK. The translation between filetypes and line ending values from Vi to Helix feels a little hacky to me here: we're basically parsing and trying to intepret Vi set commands. If we do support this in core, I'm not sure where we draw the lines for which Vi settings should be supported. And do we support interpreting emacs modelines as well?

[pascalkuthe]

vim modelines are pretty standard even in editors other than vim. Kakoune also parses these by default (https://github.com/mawww/kakoune/blob/master/rc/detection/modeline.kak). I think it would be reasonable to mirror kakoune here. It supports the following options currently:

• scrolloff
• sidescrolloff (do we even have a sepreat setting for this?
• tabstop
• shiftwidth
• textwidth
• fileformat
• filetype
• spelllang
• bom
• nobom

[doy]

yeah, i don't think this belongs in a plugin. i'm not tied to vim syntax specifically (we could also totally just do our own thing) but it feels pretty important to be able to have some way to override settings like file type that heuristics aren't always going to correctly identify (i have quite a few files that aren't identifiable by filename or shebang).

[doy]

this should resolve the issues mentioned so far (other than the extended list of options that kakoune supports - should i implement those now or can it wait?). let me know if there are any other issues!

[pascalkuthe]

@the-mikedavis was concerned about too much vim emulation here. I tend to agree, kakoune doesn't do this either. We don't need to translate file types

[doy]

fair enough - i do kind of wonder what the right answer is for files that are intended to be shared between people using vim and helix though. maybe we do want our own format in addition?

[doy]

i added a second commit which adds support for parsing a helix-specific modeline, since that is probably going to be more useful going forward. i think parsing some amount of vim modelines is useful because they are so widespread, but for configuration that doesn't exist in vim (or is different in vim, such as language names), we are going to want a different method for configuring that.

this currently uses the same toml configuration syntax we use for the configuration file, but not sure if that's what we actually want (given that my understanding is that the toml configuration file is eventually intended to go away in favor of some kind of scripted configuration - telling people to rewrite their local configuration file is different from telling them to fix modelines in every file they want to use). let me know if you think this is fine or if we can come up with something better here.

[pascalkuthe]

I wonder if we should habdle expandtab without an explicit shiftwidth. I think that isn't too rare. We could simply set the indentation to the tabwidth currently set for that document. This would match sw=0 (which we may also want to handle) and is generally what a user would expect.

One minor csbeat in both cases: If the document ks already configured to use spaces then expandtab should do nothing. That will need special representation in the ModeLine struct but IMO it's worth it since that is pretty common.

[lucasew]

There is some exoteric examples that I tested if Vim can properly detect the file type. Vim got it right in all but the FSharp example, that is expected because Vim doesn't have by default the file type definition for FSharp. In my tests the only thing that seems to break detection here is that requirement of a : at the end of the directive.

https://github.com/lucasew/nix-flake-shell/tree/c896400199f12a293ec6ee7d54b5f2e19f0f0a99/tests

[markstos]

vim modelines are pretty standard even in editors other than vim.

I don't know common it is parse vim's modelines. Gedit can parse expandtab, tabstop, shiftwidth, wrap and textwidth: https://help.gnome.org/users/gedit/stable/gedit-plugins-modelines.html.en

The Kate editor supports their own Kate-specific modelines:
https://kate-editor.org/2006/02/09/kate-modelines/#:~:text=Kate%20Part's%20modelines%20%E2%80%93%20also%20called,to%20Emacs%20and%20vim%20modelines.

But I imagine a lot of Helix's users will come from vim/neovim, which parse them.

I agree parsing vim's modelines would be a good fit for a plugin. Emacs can parse Vim's modelines using a plugin:
https://www.emacswiki.org/emacs/vim-modeline

Regarding the trailing colon. I'm one of the vim users who waw aware that's not required, so I don't put in in my own files. I'm not sure how common that is.

[doy]

okay, i think this should address all of the remaining comments - let me know what you think!

[eikenb]

Looks like this could use a rebase.

[doy]

i'll probably wait for further review before doing any more rebasing.

[the-mikedavis]

I chatted with @pascalkuthe about this a while ago and I now think this could fit well into core. Even GitHub has some basic modeline parsing for language detection https://github.com/github-linguist/linguist/blob/main/lib/linguist/strategy/modeline.rb

I'm hesitant about adding a new Helix-specific modeline style though. Was this added with a specific use-case in mind? Can Vim/Emacs style modelines work instead?

[markstos]

I see adopting Vim's modeline as a feature. Consider a team with both Vim and Helix users. Maintaining mode lines for two different editors would be a pain.

Also, there probably a lot of users who switching from Vim/Neovim to Helix, so it's also a feature to keep all those old modelines working without modification.

Only issue would be if Helix wanted to add a new file type to be supported before vim/neovim did, so there would need be some coordination to not unintentionally fork compatibility in the future.