This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2023-03-25
Channels
- # announcements (2)
- # beginners (30)
- # biff (12)
- # calva (2)
- # cider (62)
- # clerk (18)
- # clj-commons (20)
- # clojure (9)
- # clojure-europe (6)
- # clojure-switzerland (1)
- # core-logic (6)
- # datomic (4)
- # events (3)
- # fulcro (1)
- # membrane (15)
- # off-topic (16)
- # portal (24)
- # practicalli (2)
- # releases (1)
- # rewrite-clj (45)
- # shadow-cljs (14)
- # tools-build (1)
- # xtdb (4)
Ok, taking my first sips of coffee... Good question on read-string-data
... I'll start by playing in the REPL with your snippets above.
Optimally I think we would skip the whole read-string-data, but for compatibility with the old situation I just re-parsed the string that was coerced into a node
Right. Rebooting my brain. We https://github.com/clj-commons/rewrite-clj/issues/214#issuecomment-1430357842 to TokenNode. But if I understand this is causing issues with:
> In clj-kondo I have a predicate string-from-token
which just does :lines
since normally, the string is inside of :lines
but not for coerced string -> node
Because a TokenNode is not a StringNode as does not have :lines
.
This is the rewrite-clj of olden days that I once vendored: https://github.com/clj-commons/rewrite-clj/tree/f515767e4f07f27389ee2885cb459bd87d903887
and there was no explicit coercion defined here:
https://github.com/clj-commons/rewrite-clj/blob/f515767e4f07f27389ee2885cb459bd87d903887/src/rewrite_clj/node/coerce.clj
so yes, it would fall back to Object
: https://github.com/clj-commons/rewrite-clj/blob/f515767e4f07f27389ee2885cb459bd87d903887/src/rewrite_clj/node/coerce.clj#L46
my "fix" in clj-kondo is to get equality between string node from parse-string and coerce, by re-parsing the pr-str-ed string (which feels clunky)
but maybe on the way through parse-string I might as well rip out the whole read-string-data. why does rewrite-clj do this? to get multiple lines. but why does it need that?
something with edit-multiline: https://github.com/search?q=repo%3Aclj-commons%2Frewrite-clj%20%3Alines&type=code
So your question about why read-string-data
I think its intent is to preserve multi-line strings when writing out the nodes (to distinguish \n
from an actual newline, for example).
But I'd have to test my assumption that it is needed for that.
The fact that :lines
is used internally and externally means whatever we do we'd have to preserve that aspect of a string node.
it involves moving read-string-data to utils, so we don't get a cyclic dependency and wrapping the string in a string-reader so we can read it with read-string-data
Ok, I think your PR is option 2 from our https://github.com/clj-commons/rewrite-clj/issues/214#issuecomment-1430357842? But instead of mimicking the parser, just reuse the appropriate parts of the parser, which is a better way to go.
yes, but using the same implementation as parse-string so you don't use one implementation for the other, and a different one for the other
Ya, much better way to go. Also, good news: I forgot to release our previous fix, so this change to it should not annoy any rewrite-clj users.
I can transcribe this to rewrite-clj if you like. Unless you have an itch for commits to rewrite-clj, that's fine too.
Also coding is just one small part of it, right. There's the thought, discovery, support, design, etc. You are a great co-maintainer already.
@lee speaking of duplicate builds and climate change, don't you think this is a bit overkill?
19 in progress, 15 successful, and 68 queued checks
Just had a peek at workflows. The Libs Tests seem good (one job per lib we test against means 20 jobs), Native Image Tests seems ok (12 jobs to cover variants), but maybe the granularity of Unit Tests could be coarser (currently 68 jobs). In the general: I'm still kinda looking into de-duping unnecessary job runs on GHA. I want to evaluate this next: https://github.com/marketplace/actions/skip-duplicate-actions. For example, if I've just successfully run job X on source Y in a PR there is no need to rerun this job when merging the PR in source Y is unchanged by the merge.
@lee I'm testing libs in just one 1 job for babashka, it's even in the same runner as the rest of the tests
Rewrite-clj runs the full test suites of current releases of libs that use rewrite-clj. Some of these take a while to run, so parallelization helped here. Some of libs tests have historically been a bit flakey, so being able to rerun a single one after failure is nice.
everytime I dig into rewrite-clj's implementation I'm like: what a brilliant library it still is
And it was originally just written to update dependencies in lein project files! The original author never imagined it would be used for what is it used for today. Quite cool, yes.
And https://github.com/clj-commons/rewrite-clj/blob/main/README.adoc#contributors over the years!

I'll do a maint pass on rewrite-clj (make sure canaries are up to date) and then cut a release, ya?
