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

GUI installer for AppImageLauncherLite #243

Open
probonopd opened this issue Sep 21, 2019 · 15 comments
Open

GUI installer for AppImageLauncherLite #243

probonopd opened this issue Sep 21, 2019 · 15 comments
Labels
contributions welcome Users are encouraged to contribute enhancement New feature or request low priority Issue will be worked on, but all other issues have precedence

Comments

@probonopd
Copy link
Contributor

probonopd commented Sep 21, 2019

Currently the AppImageLauncherLite AppImage is not intuitive. Double-clicking it achieves nothing.

When the AppImageLauncherLite AppImage is double-clicked, a GUI installer should check whether AppImageLauncherLite is correctly installed in the system and if yes, launch it using systemd; if no, install it and then launch it using systemd. Since we are already bundling Qt this should be low overhead.

@TheAssassin
Copy link
Owner

I guess the whole point of AppImageLauncher Lite is that you need the CLI once, but only once. But don't get me wrong, I like this idea. Not a priority for me, but PRs highly welcome.

@TheAssassin TheAssassin added contributions welcome Users are encouraged to contribute enhancement New feature or request low priority Issue will be worked on, but all other issues have precedence labels Sep 21, 2019
@probonopd
Copy link
Contributor Author

After thinking about it some more, I think the best cause of action is to actually embed some dialogs into the existing AppRun bash script. Maybe using qarma?

@TheAssassin
Copy link
Owner

An alternative is to make a GUI in C++ with Qt. I don't care however it's done, as long as the UX is good.

Bloating the existing script is however a bad idea. Just make a separate one, and launch that if there's no arguments passed and e.g., $DISPLAY is not set. That way you can also test it easily.

@probonopd
Copy link
Contributor Author

That was my original thought, but the more I think about it I believe the best way is to wrap each and every step in proper error handling with GUI, hence to combine the logic that does stuff with checking error codes and using the GUI to react to them.

@TheAssassin
Copy link
Owner

Err... what?

@probonopd
Copy link
Contributor Author

probonopd commented Sep 22, 2019

Pseudo-code:

test_someting
if $RESULT != FOO ; then qarma "Question..." ; fi
do_something
if $? != 0 ; then qarma "Error..." ; fi

You see, the GUI stuff and the "business logic" need to be interwoven.

@TheAssassin
Copy link
Owner

I still think it makes sense to move any form of GUI into a second script. What you suggest also seems very verbose and hard to debug. Therefore separating it makes sense, too.

@probonopd
Copy link
Contributor Author

Do you realize that it will be orders of magnitude more work that way unless I am missing something?

@TheAssassin
Copy link
Owner

You're thinking way too simple and complicated, and that creates complexity and a lot of redundant and ugly code. That's what makes you think it's "magnitudes more of work". But that's not the case, as I will demonstrate in the following lines.

But first of all, let's have another look to the problems I see with your approach. Most notably, it ignores the fact that not every user of AppImageLauncher Lite might be on a desktop. So there's a lot more error handling required anyway. Furthermore, in your approach you could just make use of Bash's trap call, e.g., trap error_handler EXIT, where the handler is something like:

error_handler() {
    if [[ $? -ne 0 ]]; then
        if [[ "$DISPLAY" != "" ]]; then
            show_generic_gui_error_dialog
        else
            echo "generic error message"
        fi
    fi
}

Your entire idea is adding a lot of complexity and, as I said, makes the script unmaintainable. The script as it is right now is rock solid.

The entire approach of wrapping lines one by one with verbose error handling is completely wrong thinking, though.
Thinking a bit more about it, however, I came up with a bash-only solution.

The easiest way I can think of to allow for providing both CLI and UI would be to extract the install_ methods into a sourceable script, source that from the respective UI/CLI installers and then call the install steps one by one with appropriate error messages. A user isn't interested in what exactly broke when using the UI, we can still just print that to the CLI and if there's a bug have the user provide the CLI output (which hopefully won't happen). All you have to do is call the install steps in the right order, and that's something that rarely changes, so I'd be willing to have a little redundancy there. If we want to avoid that redundancy, we can even just define the install order in some single install method that calls them in the right order (disadvantage: no fine-grained error messages possible).

TL;DR: All you have to write, if done right, is to write a script that shows an initial installer UI (maybe just an "Install" button in a small dialog -- just make sure the user has to confirm that they want to install), and then just call a few methods in a defined order (or even just one, if you want to make it even more simple!), checks whether they reported success and if not displays an error message.

In fact I'm tempted to get rid of the entire shell script and code the whole CLI installer in C++. I could do that in a way then to allow for hooking up error handlers for the UI as well as for CLI. But that's not a priority either. The Lite user base is... pretty much, you. And I have tons of other things to do right now. Hence I am willing to refactor the existing script a bit to allow you to write something based on zenity, qarma or whatever, and produce a good, maintainable result quickly.

@probonopd
Copy link
Contributor Author

The easiest way I can think of to allow for providing both CLI and UI would be to extract the install_ methods into a sourceable script, source that from the respective UI/CLI installers and then call the install steps one by one with appropriate error messages

Yes, that'd be a good way of doing it.

In fact I'm tempted to get rid of the entire shell script and code the whole CLI installer in C++.

That as well ;-)

@TheAssassin
Copy link
Owner

Prepared everything for you to get started. You can start off by copying the existing CLI AppRun script and modifying it to display UI messages. The "interesting" bits have been extracted into a sourceable script.

@probonopd
Copy link
Contributor Author

Prepared everything for you to get started.

Can you give me a "hello world" in terms of what I should do? Thanks. (I better ask before I do it wrong.)

@TheAssassin
Copy link
Owner

You had some plan of patching the script by adding tons of || my_ui_tool "Error: abcdefg", hadn't you? You could just copy the existing installer script and append those messages behind every install/uninstall function call.

Also, you need to find your own way of displaying a UI installer in the first place.

There's few you can get wrong here.

@testbird
Copy link

The easiest way I can think of to allow for providing both CLI and UI would be to extract the install_ methods into a sourceable script,...

That would probably mean one script per CLI and UI backend.

The other way around there already exists a sourceable bash library that allows a script to work with all the different (G)UIs: https://github.com/BashGui/easybashgui

@DrHankM
Copy link

DrHankM commented Jun 23, 2021

https://github.com/BashGui/easybashgui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome Users are encouraged to contribute enhancement New feature or request low priority Issue will be worked on, but all other issues have precedence
Projects
None yet
Development

No branches or pull requests

4 participants