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

tests for decimal align #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

tests for decimal align #44

wants to merge 1 commit into from

Conversation

runvnc
Copy link

@runvnc runvnc commented Jul 3, 2016

Not to be merged, these are for @danyshaanan. What I am thinking would be convenient is a decimalAlign: 'left/center/right' that only gets applied to 'mostly numeric' columns. The way it works is, if this option is specified, detect whether or not its a numeric column, then if it is, basically the behavior from the align-dots module or something similar gets applied to those columns, and then run the indicated regular alignment after you get that decimal aligned output. I was thinking at first of just trying to make it where decimals were aligned and then the padding spaces go to the left (so always aligned to the right) but I think there are cases where after decimal aligning it really needs to be in the left or center rather than the right to go along with people's other formatting.

The example has the first column 'PACKAGE' that has a version number in it, my thought was to just try a standard 'isNumeric' and if its not then the decimalAlign option doesn't apply to that column (so it doesn't just look for period characters). This of course applies to when it is used as a global.

@danyshaanan
Copy link
Collaborator

Looks good!
I'll get on it :]

@runvnc
Copy link
Author

runvnc commented Jul 3, 2016

Awesome thanks!

@danyshaanan
Copy link
Collaborator

So... I thought that it would be as easy as splitting each value in a column by the . and padding each side as its own column. However, this does not take into consideration the way column width is calculated.

While the actual paddings are applied here, the width calculation starts all the way up here, and the required change has the unique property that two x chars long strings could require their column to be (2x-1) chars long, meaning all those lines in between should be modified in a way, (also cause the implication on wrapping and splitting). Over all, this is a major change we should more carefully consider before implementing. @timoxley What do you think? Is this feasible?

@timoxley
Copy link
Owner

timoxley commented Jul 4, 2016

@danyshaanan hmmm, yeah this isn't as simple as it first seems. Perhaps if we could configure preserving of whitespace for certain columns, then this could be added as a pre-columnify formatting step. That would also solve #37.

We could also look at adding more hooks to allow customising the behaviour.

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 this pull request may close these issues.

3 participants