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

fix: workaround to ensure terminal hyperlink is terminated correctly #50

Closed
wants to merge 1 commit into from

Conversation

mcwarman
Copy link
Collaborator

@mcwarman mcwarman commented Feb 26, 2024

Motivation

When testing a PR gh notifs hyperlink took over my terminal, which I raised #49 and cli/go-gh#150. This works around the issue by ensuring its terminated.

I also considered just adding printf '\e]8;;' after line 21

gh api notifications --template "$notif_template"

This would have at least stopped the command breaking the terminal, but each row was still broken

Changes

  • Add empty column
  • Always print line closure

Thoughts

To be honest i'm not sure i like it, and possibly prefer just adding the print for the time being.

@mcwarman
Copy link
Collaborator Author

@mcwarman mcwarman force-pushed the fix/ensure-links-are-terminated branch from a7e3a93 to 34d2efa Compare February 26, 2024 11:06
@quotidian-ennui
Copy link
Owner

quotidian-ennui commented Feb 26, 2024

To be honest i'm not sure i like it, and possibly prefer just adding the print for the time being.

I ran the original and "broke" the terminal, and then did a manual printf '\e]8;;' but it didn't fix the terminal (windows terminal + wsl2); but terminating each table row as you have it in the PR seems to properly work. (it is fixed if you do printf '\e]8;;\x1b\\' on the commandline afterwards).

As we have chatted and aluded to; hyperlink chars seeem to be included into the calculation for the column sizing so you end up with some quite broken layouts if the URL is not the last column.

@mcwarman
Copy link
Collaborator Author

Closing in favour of removing #51

@mcwarman mcwarman closed this Feb 26, 2024
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.

None yet

2 participants