Clojure-style discarding

reo101 commented 5 months ago

Add support for Clojure's #_ (a.k.a. discarding).

They support stacking and ignoring trailing/overstacking. See example in docs.

This could be deemed a breaking change since we no longer interpret #_ as (hashfn _).

It works by adding a new prefix to the parser, to be more specific - expands the logic for prefixes to be able to handle two-characters-wide prefixes and adds functionality for #_. It also keeps track of the number of stacked discards on each depth, applying them when dispatch-ing. The current depth depends on the calls to open-table and close-table.

I've added tests and updated the relevant docs (caused some unrelated, older changes to get included in the updated docs).

I've tried to follow CONTRIBUTING.md, please let me know if something is not right / should be done better

technomancy commented 5 months ago

Thanks for submitting this.

I have some hesitance in accepting it because I think it could make it significantly more difficult to write a correct syntax highlighter for Fennel. Before I merge it, I want to spend some time experimenting with adding support in fennel-mode, (used by me) pygments, (used by sourcehut for our official git repo hosting) pandoc, (used on fennel-lang.org) and possibly chroma (used by codeberg, another popular host for Fennel projects) to see how difficult it is.

If you want to beat me to it, that might help convince me to merge this more quickly. =)

technomancy commented 5 months ago

Oh yeah, this would also need to be supported by fnlfmt: https://git.sr.ht/~technomancy/fnlfmt

reo101 commented 5 months ago

There's also tree-sitter-fennel to touch up, I'll see what I can do there too (I have some progress there). I've been wondering though - shouldn't support for this be merged in those tools when 1.5.0 hits?

reo101 commented 5 months ago

Maybe even fennel-ls, we can set it up to send semantic tokens to indicate that a form is discarded (just like clojure-lsp does)

technomancy commented 5 months ago

shouldn't support for this be merged in those tools when 1.5.0 hits?

I don't think there's much risk of false positives firing on earlier code given that it would normally be a compilation error on earlier versions. People will want to use Emacs and fnlfmt at least on unreleased versions of Fennel. But a big part of it is not necessarily to ship the feature but to walk thru the process of implementation in order to make sure it's not adding too much work for people maintaining all the other tools to support.

Maybe even fennel-ls

Since fennel-ls uses fennel's own parser, it should get support for this automatically as long as discard-type comments get treated the way existing comments get treated, which is what we need for them to be supported in fnlfmt too.

Also wanted to say that from a technical perspective your patch is very thorough, so it's not on any technical grounds that I'm hesitating. Nice work on that.

reo101 commented 2 months ago

Hey, I've rebased on the latest main and ensured that make test succeeds again

Not really sure about the CI failure tho

reo101 commented 2 months ago

Hm, passed after regenarating the manpages 🤔

reo101 commented 2 months ago

While documenting the #_ form, I referred to it as Clojure's #_. But since it actually comes from EDN (which Clojure uses), should we call it an EDN feature instead?

technomancy commented 2 months ago

No, this feature existed in Clojure long before EDN was created.

reo101 commented 2 months ago

Some updates on my side:

I've rebased the PR onto main (two days ago), whilst also refactoring my logic a bit and making it cleaner (IMO), you're invited to prove me wrong 😄
I've managed to integrate discards into the fennel tree-sitter parser (the one used by neovim). It works great, modulo everything that is parsed using pairs (local + binding pair, let + lots of binding pairs, maps). The clojure tree-sitter parser does not enforce the pairs (it just parses everything as a sexp), but the fennel one parses the common macros and such as separate nodes with special syntax (makes common queries on those structures faster). I've also managed to make it work when we use #_ at toplevel (ignoring the subsequent toplevel form), which is something...
... I still haven't achieved here (in the real parser). I am still a bit dumbfounded as to how toplevel form parsing works (the checks for the implicit module return value still confuse me a little).

I'll rebase onto main again soon and continue pondering on how to do the toplevel discarding (would not turn away any hints 😄)

bakpakin / Fennel

Clojure-style discarding #479