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

Equal signs showing in the IntelliJ terminal on beta version #11042

Closed
aklajnert opened this issue Jan 13, 2025 · 9 comments
Closed

Equal signs showing in the IntelliJ terminal on beta version #11042

aklajnert opened this issue Jan 13, 2025 · 9 comments

Comments

@aklajnert
Copy link

aklajnert commented Jan 13, 2025

I've upgraded my fish to 4.0 beta, and noticed something weird in my PyCharm terminal. Every command starts and ends with equal sign (=). Here's how it looks:

image

Switching back to 3.7.1 solved the problem. I'm on Windows and run the terminal on WSL2.

@faho
Copy link
Member

faho commented Jan 13, 2025

Duplicate of #10829

@faho faho marked this as a duplicate of #10829 Jan 13, 2025
@faho faho closed this as completed Jan 13, 2025
@faho faho added the duplicate label Jan 13, 2025
@krobelus
Copy link
Contributor

well it's not fixed (WSL seems like a case like SSH where environment variables are not forwarded). we do have a fix if you're willing to reconsider.

(To be clear the statement "it has shown issues in MS conhost" in 8a5b1cc (Revert "Probe for kitty keyboard protocol support", 2025-01-07) is incorrect.)
The main question is, how much do we want to force users/terminals to fix their CSI parser.
It's true that a terminal might in theory react poorly to \x1b[?u but I do not think that is a realistic scenario, because unlike \x1b[=4u with the problematic =, the ? character is successfully consumed by every terminal we know of, since it's used in other commands like bracketed paste.

I don't think users should suffer because of this so, in light of this newly reported scenario I think we should re-add the kitty protocol check to the integration branch. (Or at least I don't want to be one blamed for pushing too hard for progress).
If we had a definition of what terminals we would want to test it on, we could certainly do that.

Alternatively, if we could find a way to identify IDEA terminals, we could work around it that way, but they don't support XTVERSION unfortunately

@aklajnert
Copy link
Author

Any workaround available? I see that the ticket in JetBrains tracker is open for 2 months, without any movement.

@krobelus
Copy link
Contributor

As a workaround you can export TERMINAL_EMULATOR=JetBrains-JediTerm before starting fish.
Not sure what's the value of $TERMINAL_EMULATOR in your case?

@faho
Copy link
Member

faho commented Jan 13, 2025

I don't think users should suffer because of this so, in light of this newly reported scenario I think we should re-add the kitty protocol check to the integration branch.

The current complexity of the querying makes me extremely wary of adding it to 4.0 at this point.

Not sure what's the value of $TERMINAL_EMULATOR in your case?

The issue is that WSL doesn't pass the environment variable along, so setting it outside of WSL doesn't work. You'd have to set it inside of WSL, but I think we only check for it on startup, so setting it in config.fish won't work.

$TERMINAL_EMULATOR will be unset.

We could of course add a configuration variable that you can set inside of config.fish.

@aklajnert
Copy link
Author

I've managed to set the environment variable in File | Settings | Tools | Terminal within the IDE and it works fine, thanks!

@krobelus
Copy link
Contributor

The current complexity of the querying makes me extremely wary of adding it to 4.0 at this point.

Interesting. Wwhat's the worst thing that could happen? The things I can imagine OTTOMH are

  1. no/very slow response
  2. cat malicious-file printing a query response
  3. torn query response, causing us to miss it
  4. user types compex key chords while starting fish, so they may be misinterpreted due to slowness.
  5. the terminal misinterprets the query
    All of these can happen but in 1-4 we have graceful fallback behavior (also 4 is a general problem) which seems better than an apparent regression?

@faho
Copy link
Member

faho commented Jan 13, 2025

Well, the obvious one is that the terminal misinterprets it, at which point we're no better off.

Or it can misinterpret any of the other queries that are now involved, including synchronized output and the alternate screen. Or we get weird interaction with those when e.g. the terminal is resized at the wrong moment. Or whatever - there are a lot of moving parts here and we haven't really tested any of it more than just casually using it ourselves. Remember #10663? We would have to at least redo that.

We now know the terminals that screw up when we just try to enable it, and there aren't many.

The main question is, how much do we want to force users/terminals to fix their CSI parser.

In this specific case, I think we can ask the large company maintaining a not terribly important terminal (that already has issues in their fish integration) to fix their stuff.

I would add a variable or possibly feature flag that users can use to work around this in general.

@krobelus
Copy link
Contributor

krobelus commented Jan 13, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants