bat

bat copied to clipboard

This PR is to resolve issue https://github.com/sharkdp/bat/issues/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

https://github.com/sharkdp/bat/assets/88001922/4e20a7e7-62b5-4407-8647-57287c471d08

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?

I think the CI failure will be fixed by https://github.com/sharkdp/bat/pull/2811.

Thanks for the review!

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?

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

Thank you for your contribution. [...] let's maybe make this opt-in at first?

I'm not sure if it would be a good idea to transition this to becoming opt-out in the future without more feedback from users. I don't believe it's common for terminal programs other than the shell to set the title by default; vim requires you to give permission by adding set title to its config to enable it, for instance.

There's also the chance that setting the terminal title using a fixed pattern could break someone's workflow. For example, I have tmux configured to have different key bindings depending on the foreground application, which is determined by the pane's title. It would be an easy fix for me since I'm aware of the change, but it might be frustrating to others who aren't aware of the change.

But then again, it would likely be a small minority that has that issue. At some point, improving the UX for the many outweighs breaking the obscure workflows of the few.

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.

It should be safe. OSC sequences follows the form of INTRODUCER ID ; CONTENT TERMINATOR, with the introducer being ESC ] and terminator being either BEL or ESC \. This allows the terminal emulator to know where the sequence starts and ends, which means it can ignore any commands that it doesn't recognize. A good example of this would be hyperlinks, which are implemented using an OSC sequence:

VS Code (on the left) supports and recognizes hyperlinks, while Terminal.app (on the right) does not. This also applies to any random command that doesn't exist:

Thank you for your contribution. [...] let's maybe make this opt-in at first?

I'm not sure if it would be a good idea to transition this to becoming opt-out in the future without more feedback from users. I don't believe it's common for terminal programs other than the shell to set the title by default; vim requires you to give permission by adding set title to its config to enable it, for instance.

There's also the chance that setting the terminal title using a fixed pattern could break someone's workflow. For example, I have tmux configured to have different key bindings depending on the foreground application, which is determined by the pane's title. It would be an easy fix for me since I'm aware of the change, but it might be frustrating to others who aren't aware of the change.

Good points. Let's design it as an opt-in feature.

Thanks for adding this! There are a couple of tiny changes that I would like you to make, but this looks great otherwise!

Thanks for the review! I think I have implemented the changes. I'll get the tests fixed in the next day or two