Change default algorithm in detect columns

[WindSoilder]

Description

@fdncred found another histogram based algorithm to detect columns, and rewrite it in rust: https://github.com/fdncred/guess-width

I have tested it manually, and it works good with df, docker ps, ^ps. This pr is going to use the algorithm in detect columns

Fix: #4183

The pitfall of new algorithm:

1. it may not works well if there isn't too much rows of input
2. it may not works well if the length of value is less than the header to value, e.g:

c1 c2 c3 c4 c5
a b c d e
g h i j k
g a a q d
a v c q q | detect columns

In this case, users might need to use --old --legacy to make it works well.

User-Facing Changes

User might need to add --old --legacy to scripts if they find detect columns in their scripts broken.

Tests + Formatting

Done

After Submitting

NaN

[hustcer]

--old may not be a good flag name, can we change it to --legacy or something else ?

[WindSoilder]

Sure! Just renamed it.

[fdncred]

I think if we can maybe just refactor the combine columns functionality so it works with the legacy and new algorithm, and add the examples back, we could land this.

[fdncred]

if you've added it and it you're good with it, feel free to land this. i'm too tired to study the changes tonight. LOL. 😆

[WindSoilder]

Ok, it's committed by c07395e

Let's land it and see if it works :)

[fdncred]

I love that this works so much better. Great job @WindSoilder! ❤️ Thanks for working on this with me!

[hustcer]

After trying the latest nightly build, I'm wondering if the previous algorithms may be more widely applicable? Is it a better idea to add the new algorithm as a feature with flag --guess-width rather than set it as the default algorithm and make it a breaking change?
BTW: --legacy make me think the flag is kind of deprecated or not recommend to use and will be removed someday.
Any thoughts? @WindSoilder @fdncred

[WindSoilder]

Do you have some examples show that previous algorithms works better?

[hustcer]

Do you have some examples show that previous algorithms works better?

My examples all falls into the pitfalls of the new algorithm such as:

git remote -v | detect columns -n
git diff head~2 --numstat | detect columns -n

[fdncred]

I'm personally fine with having the new algorithm as --guess and making the legacy mode default. I just thought it worked better because I hadn't found a way to break it yet. @hustcer found a way. 😆

I wish there was a better name other than --guess though. Any ideas? --alt? for alternate maybe

Another thought

What if we keep --guess as the default but if it returns only one column, we automatically try the legacy mode? Is that worth trying @WindSoilder @hustcer ?

[WindSoilder]

What if we keep --guess as the default but if it returns only one column, we automatically try the legacy mode? Is that worth trying @WindSoilder @hustcer ?

I think it's worth to try, but because we are close to next release, I would prefer to try it after next version

[hustcer]

Another thought

What if we keep --guess as the default but if it returns only one column, we automatically try the legacy mode? Is that worth trying @WindSoilder @hustcer ?

git remote -v | detect columns -n returns two columns already, but not the same as git remote -v | detect columns -n --legacy

BTW: --legacy make me think the flag is kind of deprecated or not recommend to use and will be removed someday.

If we want to keep the new algorithm as default, we'd better rename --legacy flag to a more meaningful name (like --detect-by-space or something else), so we can know the algorithm used by the flag to avoid next breaking change.