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

Cell alignment based on content like in Excel #303

Closed
Skeeve opened this issue Mar 12, 2024 · 11 comments
Closed

Cell alignment based on content like in Excel #303

Skeeve opened this issue Mar 12, 2024 · 11 comments

Comments

@Skeeve
Copy link
Contributor

Skeeve commented Mar 12, 2024

Is your feature request related to a problem? Please describe.

As stated in #299 I do not know the data I get in my app and keeping track of the "numericality" seems like duplicate work to me.

As I proposed there, maybe an automatic cell alignement similar to Excel (or any other spreadsheet program) could be useful to some.

I would have expected that text.AlignDefault would do this, but it applies to the whole column and is based on the "common denominator" of all cells in the column.

To not break compatibility, this behavior cannot be changed.

Describe the solution you'd like

I propose a new text.AutoCell alignment, which alignes cells based on its content:

  • numerical Cells: text.AlignRight
  • all others: text.AlignLeft

Of course this only works for text and html tables. Markdown, csv, and tsv won't benefit.

Describe alternatives you've considered

Alternatively everyone wanting numbers to be auto aligned right and and text in the same column left, has no choice. They have to decide for one of those.

Additional context

I already had a go at the implementation and it seems to work.

Perhaps what I have done is a little too crude for your taste, but I would like to discuss it with you.

@jedib0t
Copy link
Owner

jedib0t commented Mar 13, 2024

I haven't forgotten this suggestion from the other PR of yours. Give me some time to think about this, real-life-work is keeping me busy.

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 13, 2024

no issue. I implemented it in the proposal/AutoCellAlignment, just in case you're interested. I didn't create a pull request yet.

@jedib0t
Copy link
Owner

jedib0t commented Mar 14, 2024

Solved it a little differently to account for all usages of text.Align* in a new branch align-auto: main...align-auto --- suboptimal when AlignAuto is used in ColumnConfig as this will result in a RegExp parse for all column values, but it will have to do for now until a major version bump where I can pass in the actual object to Align instead of the stringified form.

Try it and let me know if that works for you.

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 14, 2024

Try it and let me know if that works for you.

That works for me! Thanks a lot!

as this will result in a RegExp parse for all column values

  1. Why not use ParseFloat, checking the error, like you do in sorting?
    if _, err := strconv.ParseFloat(text, 64); err == nil { works fine
  2. Did you think about sorting?

I'm currently experimenting with my fork, how to properly sort mixed-columns.

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 14, 2024

P.S.: I've implemented, as a proposal, in my sort-auto an AscAuto and DscAuto which will group Numbers before Text and sort both of them numerically resp. textually, ignoring case.

@jedib0t
Copy link
Owner

jedib0t commented Mar 14, 2024

I've fixed Sorting to take into account missing cells in a new commit on the same branch.

AscAuto and DscAuto is not the way to approach this. What if someone wants Alpha before Numbers? Will think more on this later.

@jedib0t
Copy link
Owner

jedib0t commented Mar 14, 2024

Release with fix(es): https://github.com/jedib0t/go-pretty/releases/tag/v6.5.5

@jedib0t jedib0t closed this as completed Mar 14, 2024
@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 14, 2024

AscAuto and DscAuto is not the way to approach this. What if someone wants Alpha before Numbers? Will think more on this later.

Fair enough. With breaking compatibility I would go for:

Asc, Dsc -> As is

AscNumeric, DscNumeric -> Sort Numbers before Alpha and sort both independently

AscAlpha, DscAlpha -> Sort Alpha before Numbers and sort both independently

Please also consider ignoring case when sorting. While it is logical for programmers that "a" comes after "Z", it's illogical for non-programmers which will use the programs.

Maybe add IgnoreCase bool to type SortBy?

@jedib0t
Copy link
Owner

jedib0t commented Mar 15, 2024

Maybe add IgnoreCase bool to type SortBy?

That sounds like a good feature. Want to contribute a PR? If not, I'll get it done in the near future.

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 15, 2024

I will try.

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 15, 2024

See #309

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

No branches or pull requests

2 participants