> • Type into document such that {:foo :bar} becomes {#_#:foo :bar}. > • See error pop up: Request textDocument/codeAction failed. > https://github.com/clj-commons/rewrite-clj/issues/87 > Repro: (`|` represents cursor) > clojure-lsp version used: 2022.09.20-16.33.44-nightly >

{:foo b|}

()#_() 

(|)#_()

({:foo}) ;; 💣

(/foo :bar)

(/foo :bar)    -->    (/foo) :bar

(/foo) :bar    -->    (/foo :bar)

Thanks for investing all this time to summarize your findings on this topic @U90R0EPHA.

With https://cljdoc.org/d/rewrite-clj/rewrite-clj/1.1.45/doc/user-guide#_parsing_peculiarities, rewrite-clj currently only parses valid Clojure code, but that’s at all saying that we won’t handle invalid code in the future. We would just have to invest the time in a solution that makes sense.

We might be able to steal some strategies from clj-kondo’s internalized and customized version of rewrite-clj. Not sure exactly how far it goes. It must at least deal with invalid symbols, but maybe not unbalanced braces, etc.

parinfer could possibly be used to guess where unbalanced braces should be placed and allow delineation of top level forms

interesting idea @U0BUV7XSA, deciding when and how to recover from unbalanced braces can be tricky.

yeah, but just identifying unreadable tokens similar to uneval nodes might be a great step

True, we don’t have to do it all at once. Could tackle the easier bits first.

@U90R0EPHA for your {:foo b|} example, rewrite-clj happens to support unbalanced maps today, so there might be a workable solution for that specific case. To demonstrate:

(require '[rewrite-clj.zip :as z])

(-> "{:foo b}"
    z/of-string
    z/down
    z/right
    z/remove
    z/root-string)
;; => "{:foo}"

For ()#_() are you modifying anything? (you probably are, lemme know) Rewrite-clj can read and write this structure:

(-> "()#_()"
    z/of-string
    z/root-string)
;; => "()#_()"

Same question for:

(-> "({:foo}) ;; 💣"
    z/of-string
    z/root-string)
;; => "({:foo}) ;; 💣"

I do see your /foo issue:

(-> "(/foo :bar)"
    z/of-string)
;; => ExceptionInfo Invalid symbol: /foo.  clojure.core/ex-info (core.clj:4617)

@ericdallo when you have a moment, please feel free to chime in on above too.

I see lots of possibilities here, so maybe we could prioritize some of the most common invalid codes https://github.com/clojure-lsp/clojure-lsp/blob/master/lib/src/clojure_lsp/parser.clj#L46-L99 is where clojure-lsp do its custom ugly parse 😅 @U04V15CAJ may be interested on that as well.

> or ()#_() are you modifying anything? When examples say "place cursor ...", there is no further editing required. The errors will trigger just by moving the cursor into regions specified. (Not clear to me how that ties in with what rewrite is doing. But when I asked @ericdallo, he indicated this was still more-or-less the same issue as the /foo problem I discussed with you.) Check this out for significant elaboration on those 2 examples. https://github.com/clojure-lsp/clojure-lsp/issues/1268

@U90R0EPHA thanks, watcha think @ericdallo? I don’t yet see rewrite-clj as being the problem for some examples I showed to work with rewrite-clj above. But… I totally fine with being wrong about that.

There’s some good ideas and initial discussion in https://github.com/clj-commons/rewrite-clj/issues/87, I’ll study that more carefully soon to freshen my brain on this topic.

It could be that rewrite-clj should be split into two different libraries. I've totally not thought this through, it's just something that strikes me. Some editor tooling needs quite different help from what other tooling needs in terms of Clojure source code reading and manipulation. While typing, about anything should be accepted. Other use cases wants stricter and more semantic help. Is it the same library that should cater to both?

So, personally I feel that rewrite-clj was always a text parser. If we're talking about transforming sexps tools.reader would have been sufficient. But we want to maintain whitespace and comments; things outside sexps. unreadable errors in source code happen and, for me, it would fit well in rewrite-clj. If you look at the biggest use cases: formatting, analysis, refactoring i think they would all benefit from error handling.

I haven't read all of the conversation here (having a day off), but every time someone took the effort to report a case where clj-kondo should continue parsing with invalid code instead of throwing, it's been solved. So please keep doing that.

I don’t think we need to split rewrite-clj to two different libraries. We can figure out something sensible.

Haha, yeah, maybe it is non-sensible. 😃

in clj-kondo I vendored both rewrite-clj and tools.reader upon which it depends to deal with these problems

(also to get better error messages especially from tools.reader)

Oh no, didn’t at all mean your idea was not appreciated @U0ETXRFEW! We are at the brainstorming stage and all input is great.

this was before rewrite-clj v1 was out

Yeah, I had to futz tools.reader a bit to deal with line endings, but did not inline. Yet.

you might as well use edamame and propose a feature request there if you need something weird ;)

See, all ideas are good!

I think it has the similar better error messages that the inlined tools.reader has

It’s an option.

perhaps one day I could undo the vendoring if we go that way :-D

I also vendored rewrite-clj since it was a bit unmaintained and I didn't want to be limited by that

but this is no longer true

Yeah but you had some tweaks too. Like skipping all whitespace for perf, and treating meta differently I think.

Yes. The skipping of whitespace could be easily an option in rewrite-clj though

True, true.

I mean, it doesn't really fit with the "rewrite" use case, but the tweak is very minimal

When borkdude takes a day off, how many borks join him? Do you leave a few borks behind to keep working?

I'm on the train home now... catching up ;)

Which bork am I talking to? I never know. Maybe it really doesn’t matter.

I only use rewrite-clj by proxy, so am actually not very familiar with it or informed about the boundaries and stuff it operates within.

Indirectly through clojure-lsp, clj-kondo, zprint (and formerly cljfmt)?

Yes, Those.

Formerly cljfmt?

I thought you might have switched to zprint? I dunno, maybe a false rumour.

Ah, I thought you was just telling me cljfmt had switched to something else. 😃

Oh yes, (I mean no, cljfmt still uses rewrite-clj), communication, eh? Can be tricky.

Calva is very much still using cljfmt. And also zprint.

cljfmt for formatting, and zprint for pretty printing

Interesting.

I know that zprint has many more knobs to turn, but have not ever really dug in to learn it.

I want to support zprint for formatting as well, but it gets complicated.

Maybe some day. 😃

Maybe tomorrow, maybe never.

Haha. something like that.

(an expression I learned when visiting Mexico many years ago)

I'll be stealing it.

It’s a good one, you definitely should.

> parinfer could possibly be used to guess where unbalanced braces should be placed Not sure, but Parinfer can only infer parens on correctly indented code, right?

>> parinfer could possibly be used to guess where unbalanced braces should be placed > Not sure, but Parinfer can only infer parens on correctly indented code, right? Mostly, but when I made the vim plugin, I would run paren mode to correct indentation based on parens first, then turn on infer mode that would move indentation without changing structure. I think smart mode was added later to do something similar. In my experience, at least top level forms start on col 0, so if you split the text at those points, you could possibly try parsing each chunk until you hit one you can't read then wrap it in a [:inferred (infer chunk)] node, and continue with the rest of the text. That way, linters could signal that's something wrong, but analysis or whatever could still be applied internally to that inferred node. The goal would be to read as much of the file as possible normally, and infer only that one chunk in the middle that missed a closing paren.

To mull: One thing that rewrite-clj does now (with the exception of normalizing all OS newline variants to \n) is spit out exactly what it reads in. It think it will have to do the same even in the unbalanced delimiters scenario. I also am not sure what this means for formatters like zprint, cljfmt and cljstyle. I would think they’d probably be fine if rewrite-clj started to not barf on invalid symbols and keywords, but not sure if they want to deal with code with unbalanced delimiters.