Tabular alignment #1962
Tabular alignment #1962
Conversation
That way the doc construction methods can share options such as indentation level.
Because mycpp doesn't like top-level computation (and doesn't check it statically - resulted in ASAN error)
The ANSI strings are hard-coded for now
|
Hm is the intention to do dicts as well? I think we talked about this on Zulip, but I'm not sure what the conclusion is I think it looks better for an array of numbers The nested test cases look a little weird, but maybe those are uncommon cases |
| > "dicen", "pío", | ||
| > "pío", "pío" | ||
| > ], | ||
| > [ |
There was a problem hiding this comment.
This one seems to look weirder than it used to? I wonder if having the rule only apply to "leaf" containers, i.e. containers that don't have any containers, would help
Although arguably you may want to format dicts like
{k: [1,2], k2: [3,4], ... }
etc
I'd suggest not doing it for dicts, because (i) it would need to align both the keys and the values, which might look stranger, and (ii) since you're aligning both, they'll tend to be longer, making dicts use the tabular layout less often, making it less useful and more surprising to users; and (iii) I've seen tabular alignment for lists elsewhere, but never for dicts.
You mean the formatting of I agree that |
|
Sorry for the delayed response Let me poke at this a bit more tonight, I think it's probably fine but I thought it may be weird if we do tabular alignment of lists but not dicts But I can see they are slightly different |
|
Oh one thing that could help us decide is to do the ASDL formatting, because we actually have real testdata for that, not synthetic testdata e.g. we can do Though it's also true there are no dicts in that case really! And maybe no arrays of numbers, so we can treat it like a particular use case (that we use ourselves) |
|
|
||
| max_flat_len = items[0].measure.flat | ||
| seq = [items[0]] | ||
| for item in items[1:]: |
There was a problem hiding this comment.
We are paying attention to GC objects, so I would remove the items[1:] slice and write it like
for i, item in enumerate(items):
if i != 0:
seq.append(_Text(...))
...
I use that idiom fairly often for "in between" separators, and I think it's pretty readable
| sep_width = _StrWidth(sep) | ||
| if max_flat_len + sep_width + 1 <= self.max_tabular_width: | ||
| tabular_seq = [] # type: List[MeasuredDoc] | ||
| for item in items[:-1]: |
There was a problem hiding this comment.
Ditto here, I think it makes sense to test i != n - 1 , should be about the same number of lines of code
(Or i != 0 if possible)
| second "tabular' style is used if the flat width of all items is no | ||
| greater than `self.max_tabular_width`. The third "multi line" style is | ||
| used otherwise. | ||
| """ |
There was a problem hiding this comment.
Maybe write something about why we're not using a more obvious rule? It's expensive?
e.g. as long as 2 items fit on every line, it could be tabular, etc.
e.g. if I have a screen of 80, and I have 10 strings of length no more 30, then 2 can fit on every line
I can sorta see why that requires a search, but it's probably good to document it
Use tabular alignment for printing YSH lists and BashArrays. Runs in linear time.
The behavior is to use tabular alignment so long as all elements can be printed flat within some fixed max column-width (current 15). All columns are given the same width, even if some columns could be smaller.
For examples of how this looks, see the unit tests:
https://github.com/oilshell/oil/blob/4eecd36bad9df31b2526410245272b83c6d871a3/data_lang/pretty_test.txt