Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a ColumnWidth.Contents #109

Closed
sschuberth opened this issue Jul 31, 2023 · 8 comments · Fixed by #128
Closed

Add a ColumnWidth.Contents #109

sschuberth opened this issue Jul 31, 2023 · 8 comments · Fixed by #128

Comments

@sschuberth
Copy link
Contributor

I'm proposing to add a Contents mode to ColumnWidth that works similar to Fixed, but automatically sets the width to the maximum length of text in any row of that column.

My use-case is very much related to #13: The cell contents of the grid I'm using get truncated as the terminal width is detected incorrectly, see #67 (comment). So I'd like to force the contents of some important cells to be fully shown nonetheless.

@ajalt
Copy link
Owner

ajalt commented Jul 31, 2023

It sounds like what you're asking for is already how ColumnWidth.Auto works: it tries to be as wide as its widest cell. Mordant will never create a table wider than the terminal width; even Fixed columns will shrink if they can't fit.

@sschuberth
Copy link
Contributor Author

sschuberth commented Jul 31, 2023

Actually, I tried ColumnWidth.Auto already (as it's the default), but if the terminal is too narrow to show all cell contents in full width, what happens is that contents of all cells get truncated. But I want some cells to not be truncated under any circumstances (and rather truncate some other cells earlier).

I also tried to achieve that via ColumnWidth.Expand(...), but I failed.

@ajalt
Copy link
Owner

ajalt commented Jul 31, 2023

I think I'd want to implement that as something like WeightedAuto(weight: Int), which would behave like Auto when there's enough space to fit the table, and would behave like Expand when columns need to be shrunk, but it would get priority over Auto and Expand columns if they exist. Or maybe it would be simpler to make Auto equivalent to WeightedAuto(1) and avoid adding another tier of priority. Then you could use something like WeightedAuto(99) to allocate everything to a column.

For now, the easiest way to get what you want is to probably pick a width for those columns you want to keep and use Fixed for them.

@sschuberth
Copy link
Contributor Author

For now, the easiest way to get what you want is to probably pick a width for those columns you want to keep and use Fixed for them.

Yeah, that's what I did 👍🏻

@ajalt
Copy link
Owner

ajalt commented Oct 2, 2023

After working on the design some more, I ended up generalizing all the width behaviors into a new ColumnWidth.Custom class. This enables more types of behavior than adding another special case, and ended up simplifying the implementation by removing some branches.

You can get the behavior asked for in this issue with ColumnWidth.Custom(width=null, expandWeight=null, priority=4)

@ajalt ajalt closed this as completed in #128 Oct 2, 2023
@sschuberth
Copy link
Contributor Author

You can get the behavior asked for in this issue with ColumnWidth.Custom(width=null, expandWeight=null, priority=4)

Thanks for this! However, could we get a constant for the highest priority now? It seems a bit unclean to hard-code it to 4 here.

@sschuberth
Copy link
Contributor Author

On a related note, maybe it makes sense to turn priority into an abstract property of ColumnWidth now? That way one could easily get the highest priority by max'ing over the sealed subclasses.

sschuberth added a commit to oss-review-toolkit/ort that referenced this issue Oct 4, 2023
Use a new more flexible column width option, see [1].

[1]: ajalt/mordant#109 (comment)

Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth added a commit to oss-review-toolkit/ort that referenced this issue Oct 4, 2023
Use a new more flexible column width option, see [1].

[1]: ajalt/mordant#109 (comment)

Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth added a commit to oss-review-toolkit/ort that referenced this issue Oct 4, 2023
Use a new more flexible column width option, see [1].

[1]: ajalt/mordant#109 (comment)

Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth added a commit to oss-review-toolkit/ort that referenced this issue Oct 4, 2023
Use a new more flexible column width option, see [1].

[1]: ajalt/mordant#109 (comment)

Signed-off-by: Sebastian Schuberth <[email protected]>
@ajalt
Copy link
Owner

ajalt commented Oct 4, 2023

In the next major version I'll get rid of all the subclasses and have them all be instances of Custom so you'll be able to do Fixed.priority etc. But that's a breaking change, so I'm holding off for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants