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

parse OSC1 and ignore it (set icon) #948

Closed
vancluever opened this issue Nov 23, 2023 · 6 comments · Fixed by #950
Closed

parse OSC1 and ignore it (set icon) #948

vancluever opened this issue Nov 23, 2023 · 6 comments · Fixed by #950
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor.

Comments

@vancluever
Copy link
Collaborator

Getting these in my log, even though the commands seem to be working:

xsession[321433]: warning(osc): invalid OSC command: 1;/etc/nixos
xsession[321433]: warning(osc): invalid OSC command: 8;;

Maybe some issue with parsing them that just needs to be fixed?

@mitchellh
Copy link
Contributor

Our parser is just liberal in what it accepts. The log message should probably say “unrecognized OSC command” instead. This isn’t necessarily a bug unless these are OSC commands we want to support; there are many we explicitly do not want to support (because they’re either old/replaced or just functionality we’re not interested in).

@vancluever
Copy link
Collaborator Author

Yeah I'll check into it and see what's up there; when I thought they were supported I was misreading the enum in osc.zig but it doesn't look like the command IDs and commands line up? It's weird though too because it looks like the first one is a window title change and it came up fine.

But also we might want to change the log level here to debug then to prevent log spam?

@mitchellh
Copy link
Contributor

But also we might want to change the log level here to debug then to prevent log spam?

Debug doesn't show up in release modes and it's been helpful for some people to see this to find OSC/CSI/etc. sequences that aren't supported for their workflow. If info is shown then we can switch to that. Or just change the wording to be a bit less bug-sounding.

@mitchellh
Copy link
Contributor

It's weird though too because it looks like the first one is a window title change and it came up fine.

I'm unsure in this specific case but there are some programs that send multiple sequences to do the same thing because they're unsure which sequence a terminal may support. This may be one of those scenarios, too. Uncertain.

@mitchellh
Copy link
Contributor

Looks like OSC 1 is "set icon": https://invisible-island.net/xterm/ctlseqs/ctlseqs.html That's not something I'm interested in supported but we should parse it. OSC 8 is URL related and covered by #29. We should not parse that yet and should show a log message until #29 is implemented.

@mitchellh mitchellh changed the title Invalid OSC command entries in log parse OSC1 and ignore it (set icon) Nov 23, 2023
@mitchellh mitchellh added enhancement contributor friendly A well-scoped, approachable issue for someone looking to contributor. and removed bug labels Nov 23, 2023
mitchellh added a commit that referenced this issue Nov 23, 2023
Fixes #948

We want to parse it so we can log at a lower level than warn but we
don't want to support this because it isn't very well defined.
@vancluever
Copy link
Collaborator Author

Debug doesn't show up in release modes and it's been helpful for some people to see this to find OSC/CSI/etc. sequences that aren't supported for their workflow. If info is shown then we can switch to that. Or just change the wording to be a bit less bug-sounding.

👍 1 was showing much much more than 8 anyway. Now I'm wondering what icons it was trying to set. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants