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

Consider empty cells to be not non-numeric #300

Closed
wants to merge 3 commits into from

Conversation

Skeeve
Copy link
Contributor

@Skeeve Skeeve commented Mar 10, 2024

Proposed Changes

By adding a new utility function "isEmpty", which returns true for values rendered as zero-length strings and for nil values, numerical columns will remain numerical when there are only numbers or empty cells.

Fixes #299.

Copy link

sonarcloud bot commented Mar 10, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 10, 2024

The failing test was in this table:

┌─────┬────────────┬───────────┬────────┬─────────────────────────────┐
│   # │ FIRST NAME │ LAST NAME │ SALARY │                             │
├─────┼────────────┼───────────┼────────┼─────────────────────────────┤
│   1 │ Arya       │           │   3000 │                             │
│  20 │ Jon        │           │   2000 │ You know nothing, Jon Snow! │
│ 300 │ Tyrion     │           │   5000 │                             │
│  11 │ Sansa      │           │   6000 │                             │
├─────┼────────────┼───────────┼────────┼─────────────────────────────┤
│     │            │     TOTAL │  10000 │                             │
└─────┴────────────┴───────────┴────────┴─────────────────────────────┘

Due to the last-names misssing in the column, the column is not considered to be non-numeric and thus the text "TOTAL" is right aligned instead of left aligned.

Please decide whether or not you render this an incompatibility.

@jedib0t
Copy link
Owner

jedib0t commented Mar 11, 2024

Hey. While I appreciate the effort in identifying and changing this behavior, this will result in unintended consequences like the one highlighted in the failing test.

Currently right-align can be enabled or forced using the SetColumnConfigs() interface. Auto-detection is designed to work only when all rows in a column contain numbers. Assuming <nil> or empty strings to be numeric may break existing behavior and is not worth it when there is a well-defined way to achieve the required alignment.

@jedib0t jedib0t closed this Mar 11, 2024
@Skeeve
Copy link
Contributor Author

Skeeve commented Mar 11, 2024

Is it okay for you If I open a new pull request which introduces the new property "EmptyNumeric" to ColumnConfig as described in #299?

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.

Automatic right alignment of number-cells only when number in each row
2 participants