clearlinux / clr-installer

Clear Linux* OS Installer
GNU General Public License v3.0
92 stars 42 forks source link

"clr-installer --gui" use-case confusing #683

Closed bktan8 closed 4 years ago

bktan8 commented 4 years ago

I'm in a GUI environment. When I invoke "clr-installer --gui", I was expecting the GUI-based of clr-installer to start. But instead, I got:

Incompatible flag '--gui' for the text-based installer.

mdhorn commented 4 years ago

This flag was meant to force the clr-installer-gui to run in graphical mode only.

Since the texted based binary is made from common code, the flag show there as well.

We will either hide this option of the text-based binary or have it check for the clr-installer-gui and exec that if it exists.

karthikprabhuvinod commented 4 years ago

@mdhorn : thinking about it, currently, clr-installer is the tui version of the installer so passing --gui option is the right response for it.

I think folks are confused because the term clr-installer seems generic and someone would expect a --tui or --gui to be passed to invoke a specific type of installer

The right way to address this: a.) We rename the clr-installer-gui to clr-installer. Since the exe includes both --tui and --gui functionality, we can invoke each by clr-installer --tui and clr-installer --gui b.) The tui only will then built as clr-installer-tui

I think this makes it intuitive than the previous approach as then users wont get confused anymore. For eg: Nobody will pass pass the --gui option to clr-installer-tui and be surprised

Let me know what you feel?

mdhorn commented 4 years ago

We do not want to rename binaries -- that will have wide-reaching side effects.

We will either hide this option of the text-based binary or have it check for the clr-installer-gui and exec that if it exists. If the first option of hidden the flag in the text-based binary is probably easiest.

karthikprabhuvinod commented 4 years ago

@mdhorn Hmm , even if we communicate this to devops team, documentation team etc? I was very inclined to renaming binaries as i thought if it was easy to manage it once we fix it. :(. Am I missing more concerns?

By hiding you mean, don't perform any action at all when a user run clr-installer --gui?

mdhorn commented 4 years ago

@karthikprabhuvinod There are MANY places in our build, in DevOps, Documentation, etc that will be broken if we change the name of the binaries. Many places only install the bundle clr-installer and not clr-installer-gui as we do not want to pull in all of the dependant graphics bundles.

This bug is complaining that the help show the --gui option in help. The average user should never need this flag. My suggestion and preference would be to make this an undocumented option by hiding it from the help command as follows:

diff --git a/args/args.go b/args/args.go
index b8abf34563a8..2e5cd9ae788e 100644
--- a/args/args.go
+++ b/args/args.go
@@ -218,6 +218,11 @@ func (args *Args) setCommandLineArgs() (err error) {
        flag.BoolVar(
                &args.ForceGUI, "gui", false, "Use GUI frontend",
        )
+       // We do not want this flag to be shown as part of the standard help message
+       fflag := flag.Lookup("gui")
+       if fflag != nil {
+               fflag.Hidden = true
+       }

        flag.StringSliceVarP(
                &args.Bundles, "bundles", "B", args.Bundles, "Comma-separated list of bundles to install",
@@ -338,7 +343,7 @@ func (args *Args) setCommandLineArgs() (err error) {
                &args.DemoMode, "demo", args.DemoMode, "Demonstration mode for documentation generation",
        )
        // We do not want this flag to be shown as part of the standard help message
-       fflag := flag.Lookup("demo")
+       fflag = flag.Lookup("demo")
        if fflag != nil {
                fflag.Hidden = true
        }
mdhorn commented 4 years ago

Released in 2.4.3 in Clear Linux OS 32750 or higher.