search

jedib0t / go-pretty

Table-writer and more in golang!
MIT License
2.82k stars 113 forks source link

Consider empty cells to be not non-numeric #300

Closed Skeeve closed 6 months ago

Skeeve commented 6 months ago

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.

sonarcloud[bot] commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.

Skeeve commented 6 months ago

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