zhiburt/tabled

Hyperlinks break width detection

maxbla opened this issue · 4 comments

maxbla commented

Following instructions in this gist, the way to print a hyperlink in the terminal with rust is something like format!("\x1b]8;;{}\x1b\\{}\x1b]8;;\x1b\\", display, url) which displays in the terminal as having width display.len(), but tabled appears to think its display width is approximately 14+display.len()+url.len(). This and others are listed in the operating system command section of the relevant wikipedia page.

An example of the results

┌─────────────────────────────────────────────────────────────────────────────┬────────────────────┐
│                                    name                                     │     similarity     │
├─────────────────────────────────────────────────────────────────────────────┼────────────────────┤
│ Person1 │        100         │
│         Person2         │ 65.38461538461539  │
│      Person3      │ 62.962962962962955 │
│    Person4    │ 62.962962962962955 │
│       Person5       │ 60.714285714285715 │
└─────────────────────────────────────────────────────────────────────────────┴────────────────────┘

after glancing at some docs, I think the underlying issue is with ansi-parser

zhiburt commented

Hi @maxbla thank you for opening the issue.

Could you please switch on color feature?

tabled = { version = "*", features = ["color"] }
maxbla commented

Yep, that completely fixed it. Dang, rust has a problem with features discoverability. Anyhow thanks for the library and the help.

zhiburt commented

Yep, that completely fixed it.

Great 💯

Dang, rust has a problem with features discoverability.

I'd agree

I think maybe it's worthwhile to add a ansi sequence block/comment in README.md now it has only color mentioning.

By the way If you @maxbla don't mind I'd like to hear your opinion on 2 questions.

  • what do you think about the feature name, I am just starting to think maybe ansi is a better name?
    ansi vs color

  • Do you think this feature must be turned on by default, because it could?
    I was thinking about that, but it will make library heavier by default which I think maybe a bad thing because not everyone need handling of ansi sequencee.
    What do you think?

maxbla commented

I don't like just ansi as a feature name because that's a whole standards body, who happen to have a relevant standard here. It isn't immediately clear to a library user which standard ansi refers to (ANSI C is another popular one). I think color is an okay name, since it basically communicates what is happening. It doesn't say exactly, but if you know how terminals work, you can guess that's what you need. terminal-escapes would be more accurate, but is longer and a breaking change.

As for whether the feature should be enabled by default, I think not. Of your 5 listed reverse-dependencies on crates.io, only one uses the color feature (qcow-cli) so you would be adding a significant overhead for no benefit to most of your users. You could re-evaluate your reverse dependencies later, as they might change in the future.

  • 👍1
  • ❤️1