-
Notifications
You must be signed in to change notification settings - Fork 263
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
fix(build.sh): Cosmetic spacing fix #199
Conversation
Iterm has an issue with rendering multi byte unicode chars. This commit adds an extra space to certain emojis on iterm only. This fixes current rendering issues on Linux term because that extra space was added previously unconditionally.
Alternatively the icons could be exchanged to emoji which work every where (smaller unicode points) |
/retest |
|
@@ -25,6 +25,11 @@ fi | |||
|
|||
set -eu | |||
|
|||
# Temporary fix for iTerm issue https://gitlab.com/gnachman/iterm2/issues/7901 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest replacing the 4 lines with
[[ -n "${ITERM_PROFILE:-}" ]] S=" " || S=""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated but kept the if-syntax although a bit more terse but easier to understand for non-shell gurus.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cppforlife, rhuss The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* fix(build.sh): Cosmetic spacing fix Iterm has an issue with rendering multi byte unicode chars. This commit adds an extra space to certain emojis on iterm only. This fixes current rendering issues on Linux term because that extra space was added previously unconditionally. * fix(build.sh): Avoid uninitialized variable ITERM_PROFILE
* Scripts can customize the behavior by providing their own function, plus pre/post actions. Part of knative#101. Bonuses: * moved documentation to README * added basic tests for the feature * moved the helper tests to a separate directory for clarity * refactored checking if a function exists into a function * test-infra presubmit now uses the default integration test runner * Replace last instance of "type -t" * Make custom flag smoke test a unit test * Explain test objective
Iterm has an issue with rendering multi byte unicode chars. This
commit adds an extra space to certain emojis on iterm only.
This fixes current rendering issues on Linux term because that extra
space was added previously unconditionally.