Node: util.inspect cuts off strings too early

[devsnek]

• Version: 12.2.0
• Subsystem: util/console

strings are cut off well below the limit of the tty's capacity, resulting in difficult to grok output

@BridgeAR

[BridgeAR]

@devsnek this relies upon the breakLength option which is currently set to 80 characters (it was actually at 60 before v12). I would like to set the default in relation to the terminal columns but util.inspect is used in lots of contexts, so always relying upon that might cause trouble. What we could do is to have a console specific default that checks the column size similar to our color handling in console.

[BridgeAR]

We can do something similar in the REPL.

[targos]

Having a special behavior for console or REPL output sound good to me

[silverwind]

Why not set it to Infinityon the REPL? All terminals wrap long lines and not breaking it ourself allows the terminal to re-wrap the output on terminal resize (only some terminals do that, though).

[BridgeAR]

@silverwind the output is split in a relatively natural way right now and it will improve readability for long strings, especially when the screen has a high resolution. The terminal wrapping is difficult to read and follow and has no knowledge about the actual output, so I don't think that's a good idea. I don't think that resizing the terminal is going to be a big issue. Especially if we keep the breakLength below a certain threshold.

Switching to a dynamic range between e.g., 80-120 (maybe up to 150, but lines of that length often already become harder to follow) would already improve the situation.
Most linters are configured for a maximum line length of 80 or 100 characters and that's because it's just easier to follow lines of that length.

[silverwind]

I'd agree to something like Math.min(120, tty.columns). Thought if you want to go for tty.columns, you may as well set it to Infinity because it has the same effect (until the terminal is resized, then it will get ugly).

[devsnek]

actually can this feature just be disabled by default/in console? i'd rather my terminal wrapped long items than node manually splitting the data. it makes it much harder to copy/paste and things of that nature.

[BridgeAR]

@devsnek it is possible to opt out by resetting the inspection default (e.g., util.inspect.defaultOptions.compact = true).

[devsnek]

yeah, but I think the new default is less readable and less usable. we didn't get any complaints before, and now we've already had two issues on it.

[BridgeAR]

@devsnek I think it is a bit early to jump to conclusions :) and I wouldn't consider asking for possibilities to switch as complaining.

I am happy to implement what we discussed above and I hope it'll resolve your main concern.

Other than that it's always possible to opt into using the old mode as described above (and there are multiple ways to set these settings depending on what exactly you do).

@silverwind

Thought if you want to go for tty.columns, you may as well set it to Infinity because it has the same effect (until the terminal is resized, then it will get ugly).

I would not use tty.columns directly. Instead, I would like to use a proportion (likely not linear).

[devsnek]

just another example:

"does this start with the wasm magic number" was my goal. i think most people's eyes will be immediately drawn down the first column, not across the first row. this seems like an insanely confusing way to display an array.

[cjihrig]

Linking to https://github.com/nodejs/node/issues/27915 so that related issues are tracked from here.

[cjihrig]

@devsnek if you or anyone else wants to open a PR changing the util.inspect.defaultOptions.compact default to true, I'd sign off on it. There has been a very large amount of churn in util.inspect(), and that seems to be the simplest way to deal with the issues we're encountering, without adding yet another util.inspect() option.

[BridgeAR]

Just as a heads up: I am currently looking into improving the situation in general and I'll gather some feedback during the summit.

[BridgeAR]

After talking to a couple of people at the summit I think it's best to change the long line behavior. I opened #28055 to address this issue by only splitting lines above the breakLength in case the string contains line breaks and it will only split the lines at exactly those locations.

The feedback I received was also to keep the array grouping mostly as it is: the order should not be changed, since it's otherwise not possible to copy the array into the REPL or other places anymore. We currently align all entries at the end of the string and it's likely better to align them at the start for all arrays that contain other types than numbers. I'll also open another PR to address that soon.

The issue about console.table was a regression that is already fixed, so that does not seem to be an issue anymore.

This should hopefully address all concerns about the current output.

[devsnek]

@bridgear I was saying to change the grouping not the order.

[
a, b, c,
d, e, f
]

vs

[
a,     b,
c,     d,
e,     f,
]

I think the first one is a lot more readable

[BridgeAR]

@devsnek right now the algorithm is going to try to create an ideal square with an best effort approach. It also takes the breakLength into account.

The input length often varies in size and if that's the case I use the longest entry as "spacer". In your example above the string '... 28 more items' is significantly longer than all other entries and it is therefore more space in-between the entries. I could add a special handling for the "extra entries" part but that increases the complexity of the algorithm. I guess that is what you would like to have?

[devsnek]

@BridgeAR

[
  a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t,
  u, v, w, x, y, z, aa, bb, cc, dd, ee, ff, gg, hh, ii, jj,
  ... 1337 more items
]

since we're producing output meant to be read left to right we should group it into rows not columns. imo we should also aim to keep newlines to a minimum to aid copy-pasting and take advantage of terms that can resize.

[BridgeAR]

@devsnek I'll open a PR to special handle maxArrayLength. That will automatically result in more condensed grouping. I still think we should keep the column based approach, since it's otherwise difficult to see where things start or stop, depending on the input.

[devsnek]

i don't know what maxArrayLength or "where things start or stop" mean

[targos]

@BridgeAR the main issue I see is not about maxArrayLength or column-based alignment.

Take this example:

$ node -p "Array(200).fill(0)"
[
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
                   0,                  0,                  0,                  0,
  ... 100 more items
]

All numbers take the same space, so why aren't there more of them on each row?

[BridgeAR]

I just opened a PR to address this. @devsnek @targos PTAL at #28059

[BridgeAR]

I'll open another PR on top of the already opened ones that will reduce the space between columns in case the space for the whole column is more than it has to be. I also plan on auto detecting the types of the entries of the array and and only print arrays with only numbers ordered to the right. All other arrays would be ordered to the left.

A screenshot of what I'm talking about:

[devsnek]

still.. given computers generally have more width than height, it feels like this wastes a lot of screen space and doesn't read as naturally.

[devsnek]

@BridgeAR also as another example i came upon while making a comment for your pr... the output here initially confused me and, once i understood what it was, prevented me from copying it, making me have to use console.log(_)

[BridgeAR]

I have a PR that increases the bias towards more columns and also increases the maximum columns in general. I think it's best to keep the square as rule of thumb and not increase the columns above my upcoming PR, even though I now move the columns together in case it fits best that way:

The grouping is significantly better readability wise than anything we had before and my last improvements will hopefully iron out the edges. I suggest to keep it with those improvements for a while and get some further feedback. Users will still be able to change the defaults if they want (e.g., increase the breakLength which would result in more columns).

@devsnek your last example will work as you expect with https://github.com/nodejs/node/pull/28055 applied.

[silverwind]

By the way: Is anyone else bothered by the fact that we emit single quotes on strings? Would love those to be double quotes so it would represent valid JSON (to some extend) 😉

[BridgeAR]

@silverwind I personally like single quotes a tiny bit more while I do not have a strong opinion on it. Since this is pretty off-topic to this specific issue, would you be so kind and open a new issue for that?

@devsnek this issue should be resolved due to https://github.com/nodejs/node/pull/28055 and https://github.com/nodejs/node/pull/28059. There is one more PR (https://github.com/nodejs/node/pull/28070) to change some details about array grouping further (more columns and printing output more compact at times).
If this does not address all concerns, please just reopen the issue with further details.

[LukasBombach]

If you came here searching for a way to show the hidden items in you array, you got to pass maxArrayLength: Infinity

console.log(util.inspect(value, { maxArrayLength: Infinity }));