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

Enable detecting SyncTERM DA1 response. #2743

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RealDeuce
Copy link

SyncTERM sends CSI = Pn c not CSI ? Pn c like VT terminals. This results in notcurses hanging in init.

This required WezTerm da3 support to be removed, because the SyncTERM and WezTerm responses could not both exist in the automaton. As it turns out, a dig through the WezTerm git history shows that it never actually sent CSI = ... ST in response to DA3, and notcurses took no actions in response to it, so it should be safe to remove this feature.

SyncTERM sends CSI = Pn c not CSI ? Pn c like VT terminals.
This results in notcurses hanging in init.

This required WezTerm da3 support to be removed, because the SyncTERM
and WezTerm responses could not both exist in the automaton.  As it
turns out, a dig through the WezTerm git history shows that it never
actually sent CSI = ... ST in response to DA3, and notcurses took
no actions in response to it, so it should be safe to remove this
feature.
SyncTERM has constant values after CSI =, so may as well avoid false
matches.
@dankamongmen
Copy link
Owner

is SyncTERM used? the webpage looks pretty ... unmaintainted.

good work either way, nice investigation.

@RealDeuce
Copy link
Author

Yeah, it's used (and designed) as a client for BBSs. The web page was recently (within the last month) updated to incorporate all the important feedback from the users. Even I don't use it as a general-purpose terminal emulator since it's explicitly not VT-compatible, and it doesn't support Unicode (though I may do that soon).

I whipped it up to scratch my own itch, and am constantly surprised that other people use it too. A new version will be released "Really Soon Now". Only reason I became aware of the issue is because a user accused SyncTERM of hanging. 😄

@SirCyan
Copy link

SirCyan commented May 20, 2024

Some more background, this is affecting this game:
https://dev.sqt.wtf/cgit/cyan/fernandotel/.git/tree/
Which was written as a BBS door for notcurses direct mode, but it can't run on Syncterm, which is arguably the most popular BBS terminal today.

@dankamongmen
Copy link
Owner

sorry, forgot all about this. i'll try hard to look into this asap!

@dankamongmen dankamongmen added this to the 3.1.0 milestone May 20, 2024
@dankamongmen dankamongmen self-assigned this May 20, 2024
@@ -1378,15 +1394,6 @@ da2_cb(inputctx* ictx){
return 2;
}

// weird form of Ternary Device Attributes used only by WezTerm
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure i understand why you're getting rid of the wezterm handler?

@SirCyan
Copy link

SirCyan commented May 20, 2024

I'll let Deuce chime in more with that one, but my understanding is they overlapped in the automaton (that is to say, they both couldn't be detected independently), and after reviewing the wezterm code along with its history, it looks like it never responded to DA3 with CSI = ... ST

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

Successfully merging this pull request may close these issues.

None yet

3 participants