clay icon indicating copy to clipboard operation
clay copied to clipboard

function definition not render as kind/code

Open schneiderlin opened this issue 1 year ago • 11 comments

When I have a namespace like this:

(ns some-ns
  (:require
   [scicloj.kindly.v4.kind :as kind]))

(defn add [x y]
  (+ x y))

Clay outputs the ns part, but the add function is not rendered as kind/code tools.reader seems to drop the :source field of the second form

schneiderlin avatar Sep 10 '24 10:09 schneiderlin

Hi,

Would you like to elaborate on what you mean by "Clay outputs the ns part, but the add function is not rendered as kind/code"?

Thanks.

whatacold avatar Sep 22 '24 15:09 whatacold

what I mean is like this image only the ns part is shown.

if I want (defn add ...) also visible, I need to do this image this feels weird, if I change the function definition, I need to change two places.

schneiderlin avatar Sep 23 '24 13:09 schneiderlin

Weird, I can't reproduce your problem, here is what it looks like on my side: image

Your exact code snippet also works, what version do you use?

whatacold avatar Sep 23 '24 13:09 whatacold

thank you for looking into this, I made a reproducible repo https://github.com/schneiderlin/clay-151. 2 branches in this repo, both 2-beta15 and 2-beta16 can reproduce.

schneiderlin avatar Sep 23 '24 23:09 schneiderlin

Nice! Will see if I can reproduce it using your repo.

On Tue, Sep 24, 2024 at 07:49 Lin ZiHao @.***> wrote:

thank you for looking into this, I made a reproducible repo https://github.com/schneiderlin/clay-151. 2 branches in this repo, both 2-beta15 and 2-beta16 can reproduce.

— Reply to this email directly, view it on GitHub https://github.com/scicloj/clay/issues/151#issuecomment-2369794170, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKVIOQNJWKF46L27PRB2XLZYCSJHAVCNFSM6AAAAABN6NAAXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRZG44TIMJXGA . You are receiving this because you commented.Message ID: @.***>

whatacold avatar Sep 24 '24 05:09 whatacold

@schneiderlin @whatacold Thanks, very helpful discussion.

I tried the demo and could not reproduce the problem:

image

@schneiderlin, let us look into this next time we meet?

daslu avatar Sep 24 '24 07:09 daslu

I can't reproduce it either.

P.S. Maybe we should have some logging mechanism to see what's happening while generating docs.

whatacold avatar Sep 24 '24 16:09 whatacold

Since we can't reproduce it, we can do little.

@schneiderlin Maybe you can set a breakpoint on note-to-items and see how it goes. My random guess is that hide-code? is true somehow for this defn form.

whatacold avatar Sep 24 '24 16:09 whatacold

tools.reader seems to drop the :source field of the second form

here is what I found, let me elaborate on this read-by-tools-reader get :code by https://github.com/scicloj/clay/blob/7e1fb9d0398cfface3fbee29eb3cd4b137b64fc0/src/scicloj/clay/v2/read.clj#L41

but in my environment, (defn add ...) missing :source attribute in meta

(def code "(ns macro1\r\n  (:require \r\n   [scicloj.kindly.v4.kind :as kind]))\r\n\r\n\r\n(defn add [x y]\r\n  (+ x y))\r\n")
(def forms (read-forms code))

(def form1 (first (read-forms code))) ;; ns form
(def form2 (second (read-forms code))) ;; defn add form
(meta form1) ;; => {:source     "(ns macro1\n  (:require \n   [scicloj.kindly.v4.kind :as kind]))" :line       1 :column     1 :end-line   3 :end-column 39}
(meta form2) ;; => {:line 6 :column 1 :end-line 7 :end-column 11}

schneiderlin avatar Sep 25 '24 09:09 schneiderlin

why can't reproduce

Whenever a form prefix with \r\n, tools.reader returns meta without :source, that's why only reproducible in Windows machine.

problem

Clay use clojure.tools.reader.reader-types/source-logging-push-back-reader, and this reader has inconsistent behavior when dealing with \r\n in peek-char and read-char.

the following example shows the difference between read-char and peek-char

(let [reader (clojure.tools.reader.reader-types/source-logging-push-back-reader "\r\n(+ x y)")
        _ (read-char reader) ;; consume the first \r
        p-ch (peek-char reader) ;; peek will return \n
        r-ch (read-char reader) ;; but when read return \n, it will read again, return \(
        ]
    (= p-ch r-ch)) ;; => false

this is also reported in tools.reader bug tracker

And reader use peek-char to determine if the form should log the source or not. https://github.com/clojure/tools.reader/blob/30d9f1d04417adc222ab57178dfd56d5d7a01d58/src/main/cljs/cljs/tools/reader/reader_types.clj#L8 peek-char returns \newline but in the following read-char, the \( is consumed, so the whole list form is not logged.

possible fixs

  • implement a new SourceLoggingPushbackReader, peek-char also skip \newline
  • preprocess change \r\n to \n before calling tools.reader

what would you think? @daslu

schneiderlin avatar Sep 26 '24 09:09 schneiderlin

@schneiderlin, thanks for this investigation and detailed explanation.

I will use your preprocessing advice.

Soon, we will have a better fix through #61.

daslu avatar Sep 27 '24 16:09 daslu