Start browser on server start

[kermieisinthehouse]

This will automatically start the user's default browser on any platform, opened to stash except when running under docker, as root, or when a user has passed --nobrowser.

[WithoutPants]

I personally hate this idea, but I understand why it's desirable. If we're to add it, there needs to be a config setting to override it so the user doesn't need to run --nobrowser every time they invoke it.

[kermieisinthehouse]

Agreed, I'd never use it but this is useful for new users and would resolve a small number of support requests for 'I double clicked it and nothing happened". I'll add a config option in the UI

[kermieisinthehouse]

Updated with UI-configurable option

[WithoutPants]

I think we need to consider existing users/systems in this. Existing systems should have the nobrowser flag set to true by default. Alternatively, the config flag should be flipped - startBrowserOnStartup, and default to false - then we default it to true on new systems.

[kermieisinthehouse]

I disagree actually - I think this is useful to the majority of users, which is why I made it an opt-out. I suspect that the prime stash use-case is starting it up "as needed" to reference, or just manually double clicking an executable in your downloads folder after you log in-- situations in which a browser open would be useful.

I also suspect the people who have set up service files or docker (which are excluded automatically from running) are much more likely to be able to investigate and turn it off. Either way, I'll defer to your decision

[kermieisinthehouse]

This is also going to be useful if/when we ever do desktop integration - AKA if you run ./stash from a shell, keep the current behavior of logging to stdout, but if you double click an executable, it opens without a window, opens a taskbar icon for service control, and opens your browser to see the UI.

On macOS / windows this would be a much more familiar experience - the Linux crowd can set up docker / systemd units just fine.

[WithoutPants]

Just to clarify - this is about the config key, not the executable flag.

For new users and new systems, it absolutely makes sense to open the browser when they first start it. It will reduce confusion for initial users, and will make it a more app-like experience.

It is for existing systems that I am concerned about.

Imagine a scenario where someone has set up stash to run at startup on their desktop machine. I imagine that they'd be somewhat surprised and likely a bit miffed if a browser unexpectedly appeared with their porn collection when they start their machine.

In my opinion, it's bad practice change the startup behaviour like this for existing users. Existing users are accustomed to the existing behaviour, and can easily change this behaviour if they wish. Performing this behaviour by default for existing users not only submits them to this new behaviour, but puts the onus on them to fix it, which I don't think is good UX design.

Ultimately, this is what I think is the appropriate behaviour, but I'm happy to discuss further:

• GetNoBrowserFlag should set the default value to true if there is no existing value
• when there is no config - ie it is a new system - explicitly set the nobrowser flag to true and open the browser

This means that systems created after this change will open the browser when stash is started. For systems before this change, the existing behaviour is maintained, and the user can opt-in as needed.

I think also that the way that the configuration interacts with the flags needs to be looked at as well. From memory, when a flag is provided, it persists that flag in the configuration, which we don't want it to do.

[kermieisinthehouse]

Updated to fit with last comment.

The behavior is now as follows:
On existing systems, the option is set to never open by default.
On brand new systems, it will open automatically unless it is in the conditions specified above (docker, root, etc.).
The UI option overrides.

[kermieisinthehouse]

Refactored and re-tested. Addressed all of the comments except for the separate pflags issue, and the capitalization, which was on purpose to make the flag all lowercase.

[WithoutPants]

Refactored and re-tested. Addressed all of the comments except for the separate pflags issue, and the capitalization, which was on purpose to make the flag all lowercase.

Why do we need to have the graphql field lowercase to make the flag lowercase?

[SmallCoccinelle]

It's not needed here, but imagine you had something like:

enum Flag {
  BROWSER
  DEBUG
  ...
}

ConfigResult {
    option(flag: Flag!, invert: Bool! = false): bool
    ...
}

Then you would have used a field alias:

{
    nobrowser: option(flag: BROWSER, invert: true)
    debug: options(flag: DEBUG)
    ..
}

So that's also something to consider wrt namng: the caller can invent their own names in GraphQL.

[kermieisinthehouse]

Fixed capitalization and pulled develop