Skip to content

Fix kitty integration by passig cmdline_url in the 133;C message#11203

Closed
leiserfg wants to merge 1 commit intofish-shell:masterfrom
leiserfg:fix-kitty-integration
Closed

Fix kitty integration by passig cmdline_url in the 133;C message#11203
leiserfg wants to merge 1 commit intofish-shell:masterfrom
leiserfg:fix-kitty-integration

Conversation

@leiserfg
Copy link
Copy Markdown

@leiserfg leiserfg commented Mar 2, 2025

Description

Talk about your changes here.

Fixes issue #

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@leiserfg leiserfg mentioned this pull request Mar 2, 2025
13 tasks
@adamcstephens
Copy link
Copy Markdown

Link to the kitty report here: kovidgoyal/kitty#8385

@krobelus krobelus closed this in 4378e73 Mar 3, 2025
krobelus added a commit that referenced this pull request Mar 3, 2025
Given

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes #11203

See kovidgoyal/kitty#8385 (comment)

(cherry picked from commit 4378e73)
pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
@septatrix
Copy link
Copy Markdown
Contributor

@krobelus

It's debatable whether the shell or the terminal should provide this feature but I think we should fix this regression?

Having this done by the shell has the advantage that it also works when the terminal does not have access to the procfs (e.g. VMs or SSH). A more featureful standard for that is currently being initiated by systemd: OSC 3008: Hierarchical Context Signalling

@krobelus krobelus added this to the fish 4.0.1 milestone Mar 3, 2025
@kovidgoyal
Copy link
Copy Markdown

kovidgoyal commented Mar 3, 2025 via email

@krobelus
Copy link
Copy Markdown
Contributor

krobelus commented Mar 3, 2025

to correct myself, it definitely has to go through the terminal somehow; a shell inside SSH or VM can't call notify-send on the host

@kovidgoyal
Copy link
Copy Markdown

kovidgoyal commented Mar 3, 2025 via email

pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

* completions/git: fix completions for third-party git commands

Before 798527d (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like

	set -l subcommand_args (commandline -opc)
	complete -C "git-foo $subcommand_args "

As mentioned in 3680179 (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.

The immediate goal of 798527d was to make sure we do not do any
more expansion on top of this.  To that end, it changed the above to
"\$subcommand_args".  The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.

Fix this with the same approach as in 6b5ad16 (Fix double expansion of
tokenized command line, 2025-01-19).

[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6 (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).

Closes fish-shell#11205

* Add back legacy bindings to address modifyOtherKeys regressions in iTerm2<3.5.12

As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.

Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.

We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
fish-shell#11004 (comment)).

So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd81 (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.

Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.

Fixes the problem described in
fish-shell#11192 (comment)

Closes fish-shell#11192

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

* completions/git: fix completions for third-party git commands

Before 798527d (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like

	set -l subcommand_args (commandline -opc)
	complete -C "git-foo $subcommand_args "

As mentioned in 3680179 (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.

The immediate goal of 798527d was to make sure we do not do any
more expansion on top of this.  To that end, it changed the above to
"\$subcommand_args".  The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.

Fix this with the same approach as in 6b5ad16 (Fix double expansion of
tokenized command line, 2025-01-19).

[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6 (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).

Closes fish-shell#11205

* Add back legacy bindings to address modifyOtherKeys regressions in iTerm2<3.5.12

As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.

Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.

We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
fish-shell#11004 (comment)).

So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd81 (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.

Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.

Fixes the problem described in
fish-shell#11192 (comment)

Closes fish-shell#11192

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

* completions/git: fix completions for third-party git commands

Before 798527d (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like

	set -l subcommand_args (commandline -opc)
	complete -C "git-foo $subcommand_args "

As mentioned in 3680179 (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.

The immediate goal of 798527d was to make sure we do not do any
more expansion on top of this.  To that end, it changed the above to
"\$subcommand_args".  The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.

Fix this with the same approach as in 6b5ad16 (Fix double expansion of
tokenized command line, 2025-01-19).

[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6 (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).

Closes fish-shell#11205

* Add back legacy bindings to address modifyOtherKeys regressions in iTerm2<3.5.12

As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.

Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.

We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
fish-shell#11004 (comment)).

So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd81 (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.

Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.

Fixes the problem described in
fish-shell#11192 (comment)

Closes fish-shell#11192

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

* completions/git: fix completions for third-party git commands

Before 798527d (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like

	set -l subcommand_args (commandline -opc)
	complete -C "git-foo $subcommand_args "

As mentioned in 3680179 (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.

The immediate goal of 798527d was to make sure we do not do any
more expansion on top of this.  To that end, it changed the above to
"\$subcommand_args".  The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.

Fix this with the same approach as in 6b5ad16 (Fix double expansion of
tokenized command line, 2025-01-19).

[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6 (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).

Closes fish-shell#11205

* Add back legacy bindings to address modifyOtherKeys regressions in iTerm2<3.5.12

As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.

Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.

We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
fish-shell#11004 (comment)).

So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd81 (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.

Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.

Fixes the problem described in
fish-shell#11192 (comment)

Closes fish-shell#11192

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
pull bot added a commit to tangowithfoxtrot/fish-shell that referenced this pull request Mar 3, 2025
* Remove obsolete clippy suppression

Missed this in b626943.

* Try to reduce write(3) calls for OSC 133 prompt markers

Something like

	write!(f, "foo{}bar", ...)

seems to call f.write_str() thrice.

Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.

Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer().  While at it, remove
the confused error handling.  This doesn't fail unless we are OOM (and this
new type makes that more obvious).

* Add the commandline to the OSC 133 command start

Given 

	$ cat ~/.config/kitty/kitty.conf
	notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c

kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.

The %c placeholder will be replaced by the commandline.

This is passed via the OSC 133 command start marker, kitty's fish shell
integration.

That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it.  (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)

Closes fish-shell#11203

See kovidgoyal/kitty#8385 (comment)

* completions/git: fix completions for third-party git commands

Before 798527d (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like

	set -l subcommand_args (commandline -opc)
	complete -C "git-foo $subcommand_args "

As mentioned in 3680179 (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.

The immediate goal of 798527d was to make sure we do not do any
more expansion on top of this.  To that end, it changed the above to
"\$subcommand_args".  The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.

Fix this with the same approach as in 6b5ad16 (Fix double expansion of
tokenized command line, 2025-01-19).

[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6 (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).

Closes fish-shell#11205

* Add back legacy bindings to address modifyOtherKeys regressions in iTerm2<3.5.12

As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.

Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.

We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
fish-shell#11004 (comment)).

So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd81 (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.

Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.

Fixes the problem described in
fish-shell#11192 (comment)

Closes fish-shell#11192

---------

Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants