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

Extend changes made in PR#1356 to handle vnc case and well as without vnc case. #1358

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

fghalasz
Copy link
Member

PR#1356 (by @MattHeffron) fixed medley.sh (et al) to handle --title argument with spaces. But changes only worked in cases where the --vnc option was not set. This PR extends Matt's changes to apply to the case where --vnc is set.

…guments with space; extend to handle vnc case and well as without vnc case.
@MattHeffron
Copy link
Contributor

This duplicates the default title ("Medley Interlisp") in two places: scripts/medley/medley_args.sh and run-medley. It probably should be specified in only one place. And, because run-medley sets the default, it also checks for the blank string there. (Neither implementation check for a non-empty blank string; a pathological case!)

@MattHeffron
Copy link
Contributor

These aren't show-stoppers.

@fghalasz
Copy link
Member Author

True about the title default being duplicated.

But one of my goals in developing the medley.sh scripts was that no changes should be made to run-medley. This way, we would not have to potentially update and definitely test all of the other scripts (e.g., all the loadup scripts) and workflows (e.g., Github Actions as well as all the folks that still use run-medley in their daily workflow) that currently depend on the current run-medley.

When it comes to the --title arg, the only way to set the title for VNC Viewer window is to provide an argument when you run the XVnc Server (odd but that's the way the TigerVnc code works). The window title provided by maiko (via run-medley) is ignored by the VNC setup. So to have a default title work with the VNC path in medley.sh, we need to provide a default title in medley.sh.

Bottom line: the only way to get the --title parameter to work for the VNC case and not to disturb the run-medley eco-system is to duplicate the default title in medley.sh and run-medley.

@masinter
Copy link
Member

As the original author of "run-medley" as a startup script, and it's hodge-podge of parameters, some with short and long forms and with and without '--', I'm disappointed but sympathetic that we're not able to nuke it. Ideally, though, we'd make a rename of the C entry point (the thing now called 'lde') to do what we want and get rid of all these startup scripts.

Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

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

LOTM (Looks OK To Me), not very good but ...

@masinter masinter merged commit 45513f5 into master Oct 20, 2023
@fghalasz
Copy link
Member Author

I am not convinced that adding the "UI" for medley/maiko into lde is the right way to factor things. It would be cleaner to leave lde to be the "engine" and have one or more launcher scripts/apps that deal with the arg processing and other UI and system integration things.

The launcher need not be a bash script as are medley.sh and run-medley. In fact I have a launcher 90% written in C++ and Qt that replaces both medley.sh and run-medley and provides both a GUI (on Mac, Windows, and Linux) and a command-line interface as well as providing an .il file for customizing the default args/parameters.

In fact, I would take all arg processing out of lde and run lde off of an XML file (or other easily parseable file format) passed in via stdin in the fork call from the launcher.

And I realize that Qt and C++ may be a bit controversial amongst our team. I chose Qt so I could write once and have it run without change on Mac, Windows and Linux. And C++ because it fits most naturally with Qt. But I could without a huge effort port to some other toolkit and to straight C.

At any rate, in the short term we could at least remove the dependency of medley.sh on run-medley since run-medley provides almost no value to medley.sh. But removing run-medley from all of our loadups and workflows is a bit of a bigger task and will involve some changes to some of our team's work practices.

@nbriggs
Copy link
Contributor

nbriggs commented Oct 20, 2023

No! to C++. I don't care how the arguments get passed to "lde" - but they have to be passed somehow... command line, XML, MacOS preferences, X resources (all X11 apps are supposed to honor these), whatever, but we still need to have the right collection of parameters going in to "lde", and that's the place that knows the limitations (such as multiple-of-32 for display width, and what the margins on the scroll bars are, and ... other implementation decisions that are hardcoded and we don't want to make settable).

@fghalasz
Copy link
Member Author

@nbriggs - what's your opinion on processing UI and system issues (e.g., setting MEDLEYDIR) in lde versus separating that out and doing that work in a launcher app/script.

Agreed that lde always gets the final say on interpreting (or rejecting) parameters passed in or set on the command line.

@fghalasz fghalasz deleted the fgh_update-PR1356 branch January 8, 2024 03:07
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.

4 participants