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

Commandline is needlessly repainted with new extended keyboard input functionality #10409

Closed
zx8 opened this issue Apr 3, 2024 · 4 comments
Closed
Assignees
Labels
regression Something that used to work, but was broken, especially between releases
Milestone

Comments

@zx8
Copy link

zx8 commented Apr 3, 2024

Using 3af849d and iTerm 2 (v3.4.23)

This used to work prior to the new extended keyboard input functionality, so I suspect it is related.


Define the following fish_prompt:

function fish_prompt
  echo -e '---\n$ '
end

Define the following function to be bound:

function nextdir-or-forward-word
    set -l cmd (commandline)
    if test -z "$cmd"
        nextd
        commandline -f repaint
    else
        commandline -f forward-word
    end
end

Bind it:

bind alt-right nextdir-or-forward-word

Use it:

image

@krobelus krobelus self-assigned this Apr 3, 2024
@zanchey zanchey added this to the fish next-3.x milestone Apr 4, 2024
@zanchey zanchey added the regression Something that used to work, but was broken, especially between releases label Apr 4, 2024
@zx8
Copy link
Author

zx8 commented Apr 9, 2024

@krobelus This seems to have regressed(?) since 8a7c3ce.

It is now happening again(?) with 64bc989, albeit slightly differently than originally described in the initial issue.

I can no longer reproduce with the echo a/b/c/d/e example, but I can reproduce if I recall a command from history, like so:

image

krobelus added a commit that referenced this issue Apr 9, 2024
As mentioned in 8a7c3ce (Don't abandon line after writing control sequences,
2024-04-06) we need to freshed stdout timestamps after writing to stdout
but before we might redraw, in particular when writing control sequences.

Commit a583fe7 ("commandline -f foo" to skip queue and execute immediately,
2024-04-08) made "commandline -f repaint" redraw immediately, while still
executing the bound shell command; at that time we have written "disabling"
sequences but not refreshed timestamps yet, so do that.

This is probably not needed for commands outside the repaint family.
Needless to say that this is messy, maybe we can simplify things in future.

Ref #10409 (comment)
@krobelus
Copy link
Member

krobelus commented Apr 9, 2024

thanks, fixed in 1da2087

@zanchey
Copy link
Member

zanchey commented Apr 11, 2024

Do we need a test for this?

@krobelus
Copy link
Member

Yeah a test would be good. At least on macOS it reproduces consistently, maybe even on BSD.
The implementation is very dated here, needs a proper cleanup.

vertesians pushed a commit to vertesians/fish-shell that referenced this issue Apr 17, 2024
As mentioned in 8a7c3ce (Don't abandon line after writing control sequences,
2024-04-06) we need to freshed stdout timestamps after writing to stdout
but before we might redraw, in particular when writing control sequences.

Commit a583fe7 ("commandline -f foo" to skip queue and execute immediately,
2024-04-08) made "commandline -f repaint" redraw immediately, while still
executing the bound shell command; at that time we have written "disabling"
sequences but not refreshed timestamps yet, so do that.

This is probably not needed for commands outside the repaint family.
Needless to say that this is messy, maybe we can simplify things in future.

Ref fish-shell#10409 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Something that used to work, but was broken, especially between releases
Projects
None yet
Development

No branches or pull requests

3 participants