Skip to content

Conversation

@ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Mar 14, 2022

closes #3421
also reported golang/go#51650, maybe we will get a fix in standard library at some point?

- What I did
replaced golang's tabwrittter with github.com/WeiZhang555/tabwriter to support non-latin character width

- How I did it
replace imports (github.com/WeiZhang555/tabwriter is a fork from golang's tabwriter)

- How to verify it
docker search twang2218

- Description for the changelog
docker table output supports non-latin characters to compute column sizes

- A picture of a cute animal (not mandatory but encouraged)

@ndeloof ndeloof requested a review from thaJeztah March 14, 2022 10:02
@thaJeztah
Copy link
Member

Couple of things (just from a very cursory glance);

  • the https://github.com/WeiZhang555/tabwriter package looks to be a fork of the stdlib tabwriter, but it's based on a version from 6 years ago; we should check if there's no fixes we're missing that were made since then; https://github.com/golang/go/commits/master/src/text/tabwriter/tabwriter.go)
  • I see it originally used golang.org/x/text/width, but there was an issue with that, which made it switch to use github.com/mattn/go-runewidth - slightly wondering if that was a bug in golang.org/x/text (and if so, if that either was fixed upstream, or if there's something to fix?)
  • Overall, it would be nice if golang's own text/tabwriter would do this correct "out of the box' (or if out of scope for that, if this would be something that could be part of golang.org/x/text)

@ndeloof
Copy link
Contributor Author

ndeloof commented Mar 14, 2022

I've reported golang/go#51650 to (maybe?) get some feedback on this regarding the standard library and see what would be the best option for this.

@thaJeztah
Copy link
Member

Thanks! I just spotted your comment on the ticket 😅.

So I'm a bit on the fence on whether or not we should introduce a new dependency for (possibly?) a corner-case 🤔 (as adding the dependency here, also means that other projects that use docker/cli as a module may also get it as extra dependency). Overall the dependency itself seems ok, just a bit on the fence 😅

@WeiZhang555 do you know if attempts have been made in the past fo have your fixes in upstream Go's tabwriter (and/or golang.org/x/text ?)

@rumpl
Copy link
Member

rumpl commented Mar 17, 2022

text/tabwriter is frozen, no new features and bugfixes will get into it. It also doesn't support non-latin things and never will, I think it's a great candidate to remove and use something else that fixes our issues

@ndeloof
Copy link
Contributor Author

ndeloof commented Apr 25, 2022

working on #3568 as an alternative

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Column size calculation seems off when there are non-English characters in a description

3 participants