emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: toml-ts-mode: first draft


From: Randy Taylor
Subject: Re: toml-ts-mode: first draft
Date: Tue, 13 Dec 2022 22:37:07 +0000

On Tuesday, December 13th, 2022 at 15:43, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:

On 12.12.2022 22:17, Randy Taylor wrote:
Looks good! A few silly nits:

Thanks for the constructive feedback!

- It would be nice to keep batch.sh alphabetized (so maybe move typescript while you're there).
Did this. I saw bash was missing, so I added that too. It's unrelated to TOML, but I hope it can pass :)
- Most modes put a newline between features in their font-lock rules definition. I think we should stick to that.
Done
- I think comment should be moved out of pair to its own feature.
Done
- For features like 'number, I like to group them (e.g. [(int) (float]), then you only need to specify @font-lock-number-face once.
Done
- ;;(setq global-toml-node (treesit-buffer-root-node)) seems like this was leftover debugging to be removed?
Oops. Fixed.
- treesit-font-lock-feature-list should have 4 levels, and delimiter and error should probably go in the 4th one (side note, we should all figure out the "final" list of general features and which levels they belong to). The first level should maybe just be comment on its own, the rest looks good to me.
Done.
- Indentation support for multi-line arrays would be nice (and maybe even follow the indentation of the previous line if that's not too hard and doesn't cause everything to blow up?)

I was fine with all this until you started mentioning indentation... :D

I gave it a try though, and what we have provides a customizable indentation-level, which is applied to multiline strings and array-values. (Indentation was never my "forte" if you like, and I haven't figured out an obvious way to make it follow previous line's indentation though.)

If it's OK for you, for now I would like to leave the indentation-ambitions at the point which is implemented.

Aaand...

With that said... That should (to the best of my knowledge) address everything you requested, and IMO that makes it a nice upgrade from last patch.

Attached is a patch with all changes combined up until now.

Anything else you (or anyone else) think should be fixed up?

--

Jostein

Looks good!

Just a few final comments:

- It would be nice to separate bracket out to its own bracket feature if it's not too much of a hassle. Is it not matchable just with (["[" "]"]) on its own?

- (setq-local treesit-font-lock-level 4) should probably be removed since I don't think modes shouldn't be setting that.

- Should toml-ts--indent-rules be named toml-ts-mode--indent-rules to be consistent with everything else?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]