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

Syntax highlighting aborts on enter for commands #5912

Closed
mqudsi opened this issue Jun 1, 2019 · 10 comments
Closed

Syntax highlighting aborts on enter for commands #5912

mqudsi opened this issue Jun 1, 2019 · 10 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Jun 1, 2019

If you're in a laggy environment (in my case, connecting to a tmux session over ssh - albeit on the local network, but over wifi) and type faster than fish is processing things, an <enter> after a command appears to abort the syntax highlighting process.

In this example, I typed in make followed by <enter>:

image

Naturally, the command starts off as not found and in red (m, ma, mak) but is located and turned to the regular command color as the command (make) is entered. I am hitting <enter> before fish gets a chance to update that, and the highlighter continues to display it in red.

If I were to pause for 100-200ms after typing in make before hitting enter, this would not happen.

@faho
Copy link
Member

faho commented Jun 2, 2019

type faster than fish is processing things, an after a command appears to abort the syntax highlighting process.

I'm assuming "enter" here sends \r or \n, and that that's bound to something that does execute. If so, then I'd say this is working correctly? Syntax highlighting is nice, but if it's taking a while it should be taking a back seat to, ya know, actually running stuff.

@faho faho added the RFC label Jun 2, 2019
@mqudsi
Copy link
Contributor Author

mqudsi commented Jun 9, 2019

I logged this as I'm suspicious about the whole thing. It shouldn't take long at all for fish to ascertain that make is a command, and I find it extremely unlikely that it takes longer to do so (on an SSD) than it does for me to type in a character... unless the two characters were in a single transmission that was burst received? But the network delay means it's unlikely, as that would be the opposite transmission behavior that you'd expect.

If it is indeed the case, that makes sense. I still think the terminal history is a record of what transpired and the highlighter thread should continue in the background even after execution has begun (but that can be a can of worms if the already-printed text is cleared/modified/etc by the new program being executed, so never mind).

@ridiculousfish
Copy link
Member

fish's normal syntax highlighting mode will check the filesystem to validate commands, underline paths, check redirections, etc.

It also has a zero-IO mode, which is invoked right before executing a command. In this mode all commands are assumed to be valid, intended to address exactly this issue.

So this is supposed to work, I'll investigate why it's not.

@ridiculousfish ridiculousfish self-assigned this Jun 15, 2019
@mqudsi mqudsi removed the RFC label Jun 17, 2019
@zanchey zanchey added the bug Something that's not working as intended label Aug 14, 2019
@zanchey zanchey added this to the fish-future milestone Aug 14, 2019
@ridiculousfish
Copy link
Member

Ok, we don't invoke the no-io variant of highlighting on every execution, because if the command really is invalid then we'll color it valid before failing to execute it.

What happens here is that mak is an invalid command. When you type another character we extend it to make and propagate the last character's color (error), and then kick off highlighting in the background. Before that finishes, you press enter, and we don't wait around at that point.

Note that it is not trivial to verify that make is a valid command: first the function check (perhaps autoloading) fails, then we have to explore $PATH, then we have to ensure that this user has execute privileges for the file found in $PATH, etc.

I think the right thing to do is to wait for any outstanding background highlighting to complete, but with a timeout of a few ms.

@mamiu
Copy link

mamiu commented Oct 24, 2020

I think the right thing to do is to wait for any outstanding background highlighting to complete, but with a timeout of a few ms.

@ridiculousfish Is that already possible? And if so, how?

I'm happy to accept the validation time of the command to get syntax highlighting for this use case.

@ShizuhaAki
Copy link

I still don't find #7418 duplicate with this. If so, then the problem is not just "wait for the time that fish processes colors", it is a problem regarding the order in which fish does expansions and colors the output, and, generally, the order in which fish does other things.

@mamiu
Copy link

mamiu commented Oct 27, 2020

@Ravenclaw-OIer

I think your issue is indeed related to this one. And yes you're assumption that "it is a problem regarding the order in which fish does expansions and colors the output, and, generally, the order in which fish does other things" is absolutely correct.
See here: #3031

One quote from that thread:

I'm sure there's a good reason commandline -f is currently the way it is, but the current behaviour is very unintuitive and often stills surprises me when writing scripts (and I consider myself a relatively seasoned fish user).

As programmers, unless some sort of special syntax is involved, we're used to things being executed sequentially in scripting languages, and often taking effect immediately, which makes this particular nuance all the more surprising.

@ShizuhaAki
Copy link

Well, then I think we need to compile a list of "confusing coloring" and check them one by one after we close this.

ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Nov 3, 2020
…ation

It may happen that the user types an abbreviation and then hits return.
Prior to this commit, we would perform a form of syntax highlighting
that does not require I/O, so as to not block the user. However this
could cause invalid commands to be colored as valid.

Introduce a new function which performs background I/O and then waits
for a little bit (250 milliseconds) for it to complete. If it does not
complete in time, fall back to the no-I/O variant.

This fixes fish-shell#7418; a followup commit will address the more general fish-shell#5912.
@ridiculousfish ridiculousfish modified the milestones: fish-future, fish 3.2.0 Nov 6, 2020
@ridiculousfish
Copy link
Member

ridiculousfish commented Nov 6, 2020

I believe this is solidly fixed with d3192d3. With that fix, commands wait for a little bit for outstanding syntax highlighting requests to complete, but with a timeout of 250ms. The hope is that commands will be colored correctly more often, but if syntax highlighting takes more than 250ms (e.g. an unresponsive NFS hard-mount) then we give up, color things optimistically, and execute.

Note this is strictly about syntax highlighting and does not affect the order of any fish script execution.

@mamiu
Copy link

mamiu commented Nov 6, 2020

@ridiculousfish You're awesome! Thank you so much!

Tried it right now and it's working like a charm. 🎉

This was bothering me for many years already. 🙈
It's also a fix for this issue: #7420

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

6 participants