nix-community / nix-ts-mode

An Emacs major mode for editing Nix expressions, powered by tree-sitter [maintainer=@remi-gelinas]
https://github.com/nix-community/nix-ts-mode
Other
52 stars 9 forks source link

Embed `bash-ts-mode` in multiline strings #11

Open aciceri opened 1 year ago

aciceri commented 1 year ago

As the title says, just finished a minimal proof of concept. Here a small demo, what do you think?

This is the branch where I push my experiments wit.

I created an issue instead of a PR since I still have many doubts and I would like to create a discussion:

  1. should we use a minor mode instead of the boolean variable nix-ts-mode--embed-bash
  2. assuming that all multiline strings are always bash is wrong (even if convenient 99% of time probably). Should we match things like writeShellScript instead of simply all strings between ''s? Doing so it wouldn't work anymore with plain multiline string. Personally I wouldn't like to make this work only when adding a special comment before the string (IIRC helix was doing this in the past)
  3. I tried to improve indent rules (also writing custom matchers) to make them work even when the tree contains ERROR nodes (i.e. when the user is still typing and hasn't closed delimiters like '', }, ", etc. but I concluded that it's a very difficult task. At the same time I didn't want to implement any indent rule for multiline strings contents because one should be free to indent as wanted since they contains arbitrary languages. So, when writing rules, I chose to assume that the tree doesn't contain any ERROR node. And in order to facilitate this I also added an hook that automatically closes multiline strings as soon as one types '' (if using electric-mode), also adding indentation. To be honest I'm not 100% satisfied by this solution but it's a starting point and at least it's usable (I believe).
  4. for syntax highlighting in embedded bash I'm directly mapping font lock settings from bash-ts-mode. This is a bit hacky and I believe that it the internal format changes it could break in the future. The alternative was manually copying them.

Possible improvments for 3: a. when in multiline string pressing TAB behaves as when using bash-ts-mode, I've not idea how to implement it and I don't even know how good it would work. I don't think we want that running treesit-check-indent over files from nixpkgs it gives non empty diffs. b. leaving everything as now (so no rules inside multiline strings) but move things nix-ts-mode-indent-offset spaces to the right when pressing TAB. Probably it's possible adding an advice somewhere that calls treesit-language-at-point-function, it's a bit hacky.

@remi-gelinas @marsam @purcell @antifuchs Hope I don't annoy you by tagging

remi-gelinas commented 1 year ago

Personally, knowing many people including myself write configs in Nix (Emacs configs in elisp itself is a great example), I lean on multimode packages like polymode with comment sentinels to embed other modes for my own uses.

I think this is super cool, but I'm really hesitant to apply it to all multiline strings knowing that there are many cases where it would be incorrect.

That said, I'd love some more discussion, if anyone else has any other uses for this type of embedding built directly into the mode - but at a glance it feels a bit bloaty to add to the mode by default.

aciceri commented 1 year ago

it feels a bit bloaty to add to the mode by default

Agree (mine was just an experiment, not a PR, it wasn't my intention to propose it by default)

I'm really hesitant to apply it to all multiline strings knowing that there are many cases where it would be incorrect

I agree here too but at the same time I'm not satisfied by other solutions like what I believe helix or neovim do i.e. using these injection queries provided upstream How many multiline strings are not bash usually in the average nix source? I feel that making this work only for predefined nodes we would miss a lot multiline strings.

I lean on multimode packages like polymode

I have also tried polymode but I believe that, since this is something natively possible to do with only tree-sitter in emacs then we should implement it (here). Who wants to use multimode packages is free to use them (and in general they give even more "powerful" results than what we can do here e.g. multiple lsp servers (I believe, never tried)).

How would you see a variable that let the use choose between these behaviors:

I believe I could prepare a PR for something that includes the first 3 behaviors quite easily.

aciceri commented 1 year ago

Apparently my fork introduces problems with the highlighting.

image

Using the version on MELPA it seems to work correctly.

purcell commented 1 year ago

My 2c. would be to not try to be clever about the bash regions. This is quite an early stage for the major mode, probably best to get the basics in place, like the regular indentation added in #7.

(I do wonder whether we might eventually see a polymode/mm-mode alternative tailored to treesitter modes.)