scicloj / clay

A REPL-friendly Clojure tool for notebooks and datavis
https://scicloj.github.io/clay/
Eclipse Public License 1.0
134 stars 13 forks source link

function definition not render as kind/code #151

Closed schneiderlin closed 1 month ago

schneiderlin commented 2 months ago

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

whatacold commented 1 month ago

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.

schneiderlin commented 1 month ago

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.

whatacold commented 1 month ago

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?

schneiderlin commented 1 month ago

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.

whatacold commented 1 month ago

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: @.***>

daslu commented 1 month ago

@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?

whatacold commented 1 month ago

I can't reproduce it either.

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

whatacold commented 1 month ago

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.

schneiderlin commented 1 month ago

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 commented 1 month ago

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

what would you think? @daslu

daslu commented 1 month ago

@schneiderlin, thanks for this investigation and detailed explanation.

I will use your preprocessing advice.

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