Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

function definition not render as kind/code #151

Closed
schneiderlin opened this issue Sep 10, 2024 · 11 comments
Closed

function definition not render as kind/code #151

schneiderlin opened this issue Sep 10, 2024 · 11 comments

Comments

@schneiderlin
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

whatacold commented Sep 23, 2024

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
Copy link
Contributor Author

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
Copy link
Member

whatacold commented Sep 24, 2024 via email

@daslu
Copy link
Member

daslu commented Sep 24, 2024

@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
Copy link
Member

I can't reproduce it either.

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

@whatacold
Copy link
Member

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
Copy link
Contributor Author

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

:code (-> form meta :source)

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
Copy link
Contributor Author

schneiderlin commented Sep 26, 2024

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

@daslu
Copy link
Member

daslu commented Sep 27, 2024

@schneiderlin, thanks for this investigation and detailed explanation.

I will use your preprocessing advice.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants