:pass rendered before indentation

[lread]

Hello fippsters! Thanks for your cool tool and all the hard work you've put into it!

I am new to fipp so I could very well be misunderstanding how to express myself in fipp and/or desired vs actual behavior, but here I go anyway:

Given the following code:

(require '[fipp.engine :as fe])

(fe/pprint-document
   [:group
    [:span [:pass "<"] "AA" [:pass ">"]]
    [:align
     :line
     [:span [:pass "<"] "BB" [:pass ">"]]
     :line
     [:span [:pass "<"] "CC" [:pass ">"]]
     [:align
      :line
      [:span [:pass "<"] "DD" [:pass ">"]]]]]
   {:width 6})

I get the following output:

<AA>
<  BB>
<  CC>
<    DD>

When I was more expecting something like:

<AA>
  <BB>
  <CC>
    <DD>

I noticed this while exploring using background colors in puget: greglook/puget#44.

[brandonbloom]

Very interesting! Thanks for the report.

First, I want to make clear that putting visible characters in a :pass node is a definite no-no, except for debugging as you're doing here. Very clever. I would not have thought of that.

Second, I think you've indeed found a bug. Or at the very least, a design oversight. Look at the pass node logic here:

fipp/src/fipp/engine.cljc

Lines 195 to 196 in 29f894c

	:pass
	(rf res (:text node))

and compare to the text node logic just above:

fipp/src/fipp/engine.cljc

Lines 180 to 186 in 29f894c

	(let [text (:text node)
	res* (if (zero? @column)
	(do (vswap! column + indent)
	(rf res (apply str (repeat indent \space))))
	res)]
	(vswap! column + (count text))
	(rf res* text))

The pass nodes don't handle producing the indent and instead write directly at column zero when that's the current position. I think it would be A-OK to copy/paste that logic down to the pass codepath. The rule would then be that passthrough text is still subject to the normal indenting policies. If you wanted to highlight the background of the indent, you'd need to insert your color commands prior to the newline character.

I think the code would be:

:pass
  (let [text (:text node)
        res* (if (zero? @column)
               (do (vswap! column + indent)
                   (rf res (apply str (repeat indent \space))))
               res)]
    (rf res* text))

The only difference between the text and pass node cases would be that pass skips the (vswap! column + (count text)) step. This is similar to how the escape nodes treat multiple bytes as a single byte by doing that step as (vswap! column inc). Seems pretty safe to do exactly the same logic, but with size 0. In fact, may be time those three things become a local function (fn [text width] ...).

If you want to give that a try and maybe add a test case, I'd be happy to accept that PR.

[lread]

Thanks so much for the thoughtful reply @brandonbloom!

Yes, I should have made clear that I was :passing visible chars for illustrative purposes. I actually took the inspiration from some comment code I found here:

fipp/src/fipp/engine.cljc

Lines 292 to 294 in 29f894c

	(pprint-document
	[:group [:pass "<"] "AB" [:pass ">"] :line "B" :line "C"]
	{:width 6}))

It was very kind of you to walkthrough and explain the code, I would be happy to take a stab at a PR.

[brandonbloom]

Ha! I said I would never have thought of that, but apparently I had thought of it previously :)

[lread]

Hey, at least there's comfort that you found it very clever! :-)