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

2783 setting terminal title #2807

Merged
merged 33 commits into from
Feb 23, 2024

Conversation

Oliver-Looney
Copy link
Contributor

@Oliver-Looney Oliver-Looney commented Dec 18, 2023

This PR is to resolve issue #2783

This change sets the terminal title to the input file names when the pager is being used & --set-terminal-title flag is used, to make this feature opt in

Screen.Recording.2024-01-27.at.14.34.07.mov

@Oliver-Looney
Copy link
Contributor Author

I am not sure how to fix this failing action CICD / arm-unknown-linux-gnueabihf (ubuntu-20.04) (pull_request)



Error:

With the provided path, there will be 1 file uploaded
Artifact name is valid!
Root directory input is valid!
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

How to fix this?

@Enselic
Copy link
Collaborator

Enselic commented Dec 25, 2023

I think the CI failure will be fixed by #2811.

Copy link
Collaborator

@Enselic Enselic left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to create a PR. I skimmed it over and have one initial question.

input_names += ", ";
}
}
print!("\x1b]2;{}\x07", input_names);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this behave well in all major terminal emulators on all major OSes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe ANSI escape characters are standard across terminal emulators on different OSes, https://en.wikipedia.org/wiki/ANSI_escape_code#OSC_(Operating_System_Command)_sequences

I have updated the 2 in this line to 0 to match those docs

@Oliver-Looney
Copy link
Contributor Author

Thanks for the review!

@sharkdp
Copy link
Owner

sharkdp commented Jan 17, 2024

Thank you for your contribution. I would also be worried about portability of this specific ANSI escape code. If we can not test this across a wide range of terminal emulators at the moment, let's maybe make this opt-in at first?

@Oliver-Looney
Copy link
Contributor Author

Yes, that makes sense. I'll update this pr to make this opt-in

This was referenced Jan 16, 2025
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.

4 participants