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

Replace unicode character in cylc show output #6432

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Oct 18, 2024

For some reason, the U+2999 "Dotted Fence" character is not displaying on my RHEL9 terminal despite working before in RHEL7.

image

My font is JetBrains Mono, but changing back to the default font doesn't help.

Replaced with U+2506 "Box Drawings Light Triple Dash Vertical" as it seems to be more widespread.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • No dependency changes
  • Tests updated
  • No changelog entry as minor
  • No docs needed
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly an improvement anyway, in terms of how it looks.

@MetRonnie MetRonnie changed the title Replace unicode character Replace unicode character in cylc show output Oct 21, 2024
@oliver-sanders
Copy link
Member

oliver-sanders commented Oct 21, 2024

I don't have this issue in the same RHEL 9 environment?

[edit] Neither does Ronnie now, probably fixed by a system update.

Happy to change, but would like to get a handle on what chars are likely to be better supported before fixing one environment and breaking another.

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked it up.

The previous icon was Unicode v3 (added in the 2000s), this PR changes it to an icon in Unicode v1 (added in the 90s).

So, I guess, try to keep to older Unicode icons (have just checked the Tui icons, they are all v1).

@oliver-sanders
Copy link
Member

@MetRonnie, to fix the test failure:

# update the screenshot
CYLC_UPDATE_SCREENSHOTS=true pytest tests/integration/tui/test_show.py::test_show

# test the result is stable
pytest tests/integration/tui/test_show.py::test_show

git add -u; ...

@MetRonnie MetRonnie added this to the 8.3.6 milestone Oct 21, 2024
@MetRonnie MetRonnie self-assigned this Oct 21, 2024
@MetRonnie MetRonnie marked this pull request as ready for review October 21, 2024 15:53
@hjoliver
Copy link
Member

(Unrelated flaky macos test fail)

@hjoliver hjoliver merged commit 6005ab5 into cylc:8.3.x Oct 21, 2024
26 of 27 checks passed
@MetRonnie MetRonnie deleted the unicode branch October 22, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants