Clojurians Log v2

This is correct since Selmer (at least at the time) couldn’t be run from source. You can force loading from source by using :reload

Should bb upgrade to a newer version of Selmer? I wasn’t aware there was one

Yes, please.

cool. what's the new feature?

I mean that you are particularly interested in

so I can make sure it's going to work

I'm watching Github releases but @yogthos isn't using those. Perhaps mention new releases in #releases :)

I'm getting some test failures so I'm having to go look into those soon-ish

@alberto.miorin can you please make a Github issue about this?

expected: (= "No filter defined with the name 'woot'" (try (render "{{blah|safe|woot" {:blah "woot"}) (catch Exception ex (.getMessage ex))))
  actual: java.lang.OutOfMemoryError: Required array length 2147483644 + 4 is too large

could be an error in my config

ah it fails since version 1.12.63

Oh I see, when upgrading my local test file with these changes, stuff works again. https://github.com/yogthos/Selmer/pull/315/files?w=1#diff-6199cc6a8e83f638dafce7048427f5c586303b49d1f5314d5caaf9c9e08ce702 Isn't it kind of bad that the old tests stopped working with those changes?

also, why convert a string to a char array, can't this break all kinds of unicode stuff?

Oh yeah some tests need to be updated, some of the date formatting changed, so the tests fail on newer JVM versions

No, I mean: the old tests fail horribly with those new changes.

In terms of changes, the changelog.md in the repo has the list, there's nothing major, just little quality of life things

e.g. this one:

(is (= "true" (render "{{xs|empty?" {:xs []})))

Ohh

and this one:

(render "{{blah|safe|woot" {:blah "woot"})

maybe those were typos in the old tests, but it seems Selmer is now spinning in some memory-exhausting loop while processing these templates

yeah it does look like the tests had typos in them, so might've not been exercised properly before

nothing jumps out at me that should cause a loop though, is something bb specific happening here?

stuff seems ok on the regular jvm https://github.com/yogthos/Selmer/actions/runs/18473777899/job/52633542903

but to be fair, I haven't profiled that

@yogthos My point is: I still had the old tests around in my local bb repo. I ran the updated Selmer lib against that. I causes out of memory issues that weren't these before. Demo: https://github.com/borkdude/Selmer/commit/8c08ae10959c6c48dbd83888617314f7d2205be7 Just try running the tests with these changes, you'll see what I mean

or watch this CI run: https://github.com/borkdude/Selmer/actions/runs/18620752679/job/53091583109

This should probably throw with an EOF error. Let me check if there's an appropriate location where this isn't handled...

oh yeah, looks like it tries to keep reading when it should just error out

I located it to read-template

ah found it

read-tag-conten is spinning in a loop

because nil isn't handled

that would do it, I guess just need to update the condition here (when (and (not= *tag-close* ch) (not= *filter-close* ch))?

does this work?

(defn read-tag-content [rdr]
  (->buf [buf]
         (let [next-ch (peek-rdr rdr)
               filter? (not= *tag-second* next-ch)]
           (.append buf *tag-open*)
           (when next-ch
             (loop [ch (read-char rdr)]
               (.append buf ch)
               (when (and ch (not= *tag-close* ch) (not= *filter-close* ch))
                 (recur (read-char rdr))))
             (when filter?
               (.append buf (read-char rdr)))))))

well yes, it fixes the never ending loop but it doesn't make the tests pass. do you think you should still support {{ ... or else I suggest throwing an EOF exception

and why the heck did this work before?

lol I was just gonna say, I'm surprised this never came up

and I guess supporting {{ might be the more correct thing to do since it just just be treated as regular text if no closing tag is found

I now did this:

ERROR in (emptiness) (core.clj:4739)
expected: (= "true" (render "{{xs|empty?" {:xs []}))
  actual: clojure.lang.ExceptionInfo: Expected tag-close or filter-close

yeah I think that's actually fine

what is fine exactly?

I'd go with the error

I'll make a PR

awesome thanks!

but my point in this whole thread is: when someone makes a PR to introduce a new feature, but edits old tests that used to work, that's kind of suspicious?

yeah for sure, that one's on me cause I saw the test fixed the typo, but didn't think through the implications there

the old test was testing the negative case

what about this test:

(loop []
               (let [ch (read-char rdr)]
                 (cond
                   (or (= *tag-close* ch)
                       (= *filter-close* ch))
                   (.append buf ch)
                   (nil? ch)
                   (throw (ex-info "Expected tag-close or filter-close" {}))
                   :else (do (.append buf ch)
                             (recur)))))

eh fix I mean

the pr code seemed innocuous enough

I would've at least expected a test to test the new feature :)

yeah I like that cause it gives an indication of what went wrong

tag close or filter close is actually the same character? ;)

o wait, it's dynamic

actually we might as well dump the content of the buf so far in the error too

👍

oh the tag characters can be customized

there's an options map you can pass in with custom delimiters

"Expected tag-close or filter-close: {{blah|safe|woot"

I'll make it this:

"Expected *tag-close* or *filter-close*: "

+ buf

that should give enough breadcrumbs to figure out what went wrong

oh might as well write the tag characters

could do

hmm:

"Expected } or }: {{blah|safe|woot"

when they are the same, it looks weird

oh true, maybe can just say closing tag missing then, since the buffer tells you what the opening tag was

will do

you can squash the commits if you want: https://github.com/yogthos/Selmer/pull/317

perfect, I'll push out a new version

👍 I'll bump that one in bb then, thanks a lot for the quick turnaround

no prob, and thanks for digging in!

And now coming back to @alberto.miorin’s original post: upgrade was merged to master. You can download / install the master version with:

bash <(curl )  --dev-build --dir /tmp

Thx 🙏