[inspect] Right-align indices when rendering indexed collections #202
Conversation
alexander-yakushev
force-pushed
the
inspector-align
branch
2 times, most recently
from
c1956ad to
5c2a1b0
Compare
src/orchard/inspect.clj
Outdated
| @@ -408,17 +408,31 @@ | |||
| inspector | |||
| mappable)) | |||
|
|
|||
| (defn- left-pad | |||
| "TODO: should extract this into a library." | |||
There was a problem hiding this comment.
oh no! 😄
Looks like (String/format "%10s" (object-array ["foo"])) ensures a given string has N characters, left-padding with spaces otherwise.
There was a problem hiding this comment.
Interesting, I've tried googling for it and found answers that String.format can only pad with zeros.
Still, I feel a bit reluctant to construct a format string and then use the slow formatting method for each iteration, seems a bit wasteful. Even though it will never be used for that many items anyway.
Yeah, it's probably nothing.
| `chunk` must be finite." | ||
| ([inspector chunk] (render-indexed-values inspector chunk 0)) | ||
| ([inspector chunk idx-starts-from] | ||
| (let [n (count chunk) |
There was a problem hiding this comment.
May it make sense to use bounded-count, when available, for safety?
An alternative being asserting that the chunk is Counted
There was a problem hiding this comment.
The current version of this function goes over the whole sequence non-conditionally anyway. Adding count doesn't really make it less safe. There is currently only one caller of this function that passes in a finite sequence. I renamed the argument to make this more apparent at least documentation-wise. Don't know if it needs any other guardrails.
| '(:newline)) | ||
| (render (inspect/start (inspect/fresh) (vec (range 11)))))) | ||
|
|
||
| (let [rendered (-> (inspect/fresh) |
There was a problem hiding this comment.
I'd appreciate a testing here to understand how this test is different
alexander-yakushev
force-pushed
the
inspector-align
branch
from
123084d to
98488a5
Compare
|
Ready. |
alexander-yakushev
force-pushed
the
inspector-align
branch
from
444f191 to
ca355f9
Compare
When inspecting a linear collection, the indices cause an ugly indentation shift on digit promotions (9->10, 99->100). This PR fixes this. Here's now it looks currently:
Here's how it looks after the patch:
In the future, we might want to do the similar for associative collections, but I think it's a more contentious topic. Some people don't like their values aligned to keys if it causes a large gap on the other side (me included). But aligning indices shouldn't be controversial since most of the time it's gonna be 1 space, 2 spaces at the very most in the edge case.