Add file name to progress output of tool when passed multiple files and invoked without watch mode

[aviflax]

In watch mode, the tool always outputs the name of the changed file just before processing it.

Currently, outside of watch mode, it does not. So there’s no way to tell which files have errors:

$ fc4 -fs *.yaml 
reading+parsing+validating...✅ formatting...✅ snapping...✅
reading+parsing+validating...✅ formatting...✅ snapping...✅
reading+parsing+validating...✅ formatting...🚨class java.lang.Character cannot be cast to class java.util.Map$Entry (java.lang.Character and java.util.Map$Entry are in module java.base of loader 'bootstrap')
reading+parsing+validating...✅ formatting...✅ snapping...✅

So let’s output the file names in both modes — immediate mode and watch mode. It’s a good thing anyway, all things being equal, for the output of the tool to be as consistent as possible, across modes.

[aviflax]

What about something like this?

diff --git a/tool/src/main/fc4/io/cli/main.clj b/tool/src/main/fc4/io/cli/main.clj
index 2170369..69d71c6 100644
--- a/tool/src/main/fc4/io/cli/main.clj
+++ b/tool/src/main/fc4/io/cli/main.clj
@@ -101,14 +101,14 @@
          result#)))
 
 (defn- process-file
-  [file-path renderer {:keys [format snap render watch] :as options}]
+  [file-path preamble renderer {:keys [format snap render watch] :as options}]
   ;; Optimization opportunity: this is a little inefficient in that if rendering is specified along
   ;; with formatting and/or snapping then we’re reading the YAML file twice. So we should probably
   ;; refactor render-diagram-file to accept diagram-yaml rather than a file-path. (It accepts a
   ;; file-path for historical reasons, so that the old edit workflow could call it directly. Since
   ;; we’ve removed the old edit workflow, we can change it.) (Avi Flax, July 2019)
   (try
-    (print-now "reading+parsing+validating...")
+    (print-now preamble " → reading+parsing+validating...")
     (let [yaml-file-contents (read-text-file file-path)]
       ;; Optimization opportunity: the YAML is parsed by both validate and below if format and/or
       ;; snap are true. (Avi Flax, July 2019)
@@ -150,10 +150,9 @@
 (defn- start
   [renderer {paths                       :arguments
              {:keys [watch] :as options} :options}]
-  (let [f #(process-file % renderer options)]
-    (if watch
-      (block (watch/start f paths))
-      (run! f paths))))
+  (if watch
+    (block (watch/start (fn [path preamble] (process-file path preamble renderer options)) paths))
+    (run!               (fn [path]          (process-file path path     renderer options)) paths)))
 
 (defn -main
   [& args]
diff --git a/tool/src/main/fc4/io/watch.clj b/tool/src/main/fc4/io/watch.clj
index e435cc0..20d961b 100644
--- a/tool/src/main/fc4/io/watch.clj
+++ b/tool/src/main/fc4/io/watch.clj
@@ -58,8 +58,7 @@
        " "
        (event-kind->past-tense event-kind)
        (when (> (secs-since event-ts) secs-threshold-to-print-event-time)
-         (str " at " (remove-nanos event-ts)))
-       "; "))
+         (str " at " (remove-nanos event-ts)))))
 
 (defn process-fs-event
   [{:keys [active-set process-fn executor out] :as context}
@@ -69,9 +68,8 @@
     (.execute executor
               (fn []
                 (binding [*out* out] ; See comment on :out in start.
-                  (print-now (event-preamble event-ts kind file))
                   (try
-                    (process-fn file)
+                    (process-fn file (event-preamble event-ts kind file))
                     (finally
                       (swap! active-set disj file))))))
     (assoc-in context [:recent-set file] event-ts)))

[j-po]

I was thinking about something like that, albeit passing a preamble in both cases (and briefly a shared event-preamble function, but that felt like overengineering for little savings)

[aviflax]

OK, cool.

(Yeah, re-reading the diff now, I see that I duplicated the anonymous function just so I could continue to use run! in the falsy branch of the if — I like run! for cases like these, but the fact that it accepts only a single-arity function made it awkward in this case. So were I working on this, I’d probably make the anonymous function a two-arity function, pass it over to watch/start in the truthy branch, and change the falsy branch to use doseq as you did already in one of your WIP commits, and then simply invoke the function within the doseq, supplying the path as the first arg and again as the second arg.

Not sure that made any sense… this is kinda like pair programming via carrier pigeon 😬 I have no real point, so please do ignore the above.)

[aviflax]

Whoops, just realized I posted the above diff to this issue rather than its corresponding PR #184, fragmenting the discussion — sorry about that!

[aviflax]

For future reference, the above diff was meant to be a follow-up to this comment.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.