Support reader conditionals #194
Conversation
arrdem
force-pushed
the
feature/reader-conditionals
branch
from
98cb4fe to
c7f0647
Compare
|
An alternate patch is diff --git a/kibit/src/kibit/check.clj b/kibit/src/kibit/check.clj
index 0bb373e..4870d94 100644
--- a/kibit/src/kibit/check.clj
+++ b/kibit/src/kibit/check.clj
@@ -55,22 +55,28 @@ into the namespace."
ns)
(def eof (Object.))
+(def reader-cond (Object.))
(defn read-file
"Generate a lazy sequence of top level forms from a
LineNumberingPushbackReader"
[^LineNumberingPushbackReader r init-ns]
- (let [do-read (fn do-read [ns]
- (lazy-seq
- (let [form (binding [*ns* ns]
- (read r false eof))
- [ns? new-ns k] (when (sequential? form) form)
- ns (if (and (symbol? new-ns)
- (or (= ns? 'ns) (= ns? 'in-ns)))
- (careful-refer (create-ns new-ns))
- ns)]
- (when-not (= form eof)
- (cons form (do-read ns))))))]
+ (let [do-read
+ (fn do-read [ns]
+ (lazy-seq
+ (let [form (binding [*ns* ns]
+ (try (read r false eof)
+ (catch Exception e
+ reader-cond)))
+ [ns? new-ns k] (when (sequential? form) form)
+ ns (if (and (symbol? new-ns)
+ (or (= ns? 'ns) (= ns? 'in-ns)))
+ (careful-refer (create-ns new-ns))
+ ns)]
+ (when-not (= form eof)
+ (if (= form reader-cond)
+ (do-read ns)
+ (cons form (do-read ns)))))))]
(do-read (careful-refer (create-ns init-ns)))))
;; ### Analyzing the pieceswhich just masks reader exceptions and returns a sentinel object so that we skip the form containing the reader conditional and keep going. |
|
Just saw this, I actually had just come to the same result as your first fix. I'd personally consider desirable behavior to fully support reader conditionals, as per your first fix, and fix the --remove functionality separately, disabling it again for now if necessary. I feel like being able to properly provide suggestions for all files (including those with reader conditionals) is more essential towards fulfilling the purpose of kibit than automatic replacement, which users have lived without thus far and the several bugs I reported. |
|
Yet another option here is to use the "preserve reader conditionals" mode for |
|
So I don't think that the "right" thing to do here is to teach kibit about reader conditionals at all. At best, I think that reading in a conditional-preserving mode which happens to probably prevent most kibit rewrites is the most sane thing to do next to just skipping forms containing reader conditionals entirely. |
This makes reader conditionals at least sort of work. The problem here is that because reader conditionals are correctly supported, kibit with
--replacewill now rewrite reader conditionals with their expanded form(s) which is quite likely undesired.Not sure what better behavior looks like here. It may be that we can trap the reader conditional exception and just skip the top level form on it, although this renders kibit pretty useless on namespaces with lots of conditional forms.
Fixes #191 sorta kinda maybe not the way we want.