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

add --window-title options with %M and %S variable replacement #606

Closed
wants to merge 1 commit into from
Closed

Conversation

beango1
Copy link
Contributor

@beango1 beango1 commented Jun 20, 2019

solves issue #120
continuation of #573

@rom1v
Copy link
Collaborator

rom1v commented Jun 21, 2019

Thank you for your PR. It works 👍

However, I'm wondering if we shouldn't replace the variables on the server side instead:

  • their value are device-specific values, known by the device;
  • all variables are not always used (typically, they are not), so it's useless to transmit them all (more will be available in the future, see [Suggestion] Put user-friendly name of device in window name #601) just to ignore them on the client side;
  • implementing a new variable would not require to modify the protocol between the server and the client;
  • Java is much safer for string manipulation.

What do you think?

@beango1
Copy link
Contributor Author

beango1 commented Jun 21, 2019 via email

@rom1v
Copy link
Collaborator

rom1v commented Jun 21, 2019

After doing all of the changes, I think it would have been cleaner just to set the window title the vanilla way.

A first separate commit doing just that would be great 😉

@beango1 beango1 closed this Jun 23, 2019
@beango1
Copy link
Contributor Author

beango1 commented Jun 23, 2019

continued #614

@rom1v rom1v mentioned this pull request Sep 19, 2020
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.

2 participants