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

Use the calculated path in help message #146

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

Arclite
Copy link
Contributor

@Arclite Arclite commented Mar 27, 2022

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Problem/Feature

Running the twitch command without any other arguments prints some help information. This help information includes this section:

Flags:
      --config string   config file (default is $HOME/.twitch-cli/.twitch-cli.env)

The path that is listed here appears to be a legacy value—on my machine it is incorrect. (macOS; the path is actually $HOME/Library/Application Support/twitch-cli/.twitch-cli.env).

Description of Changes:

This PR changes the help for the command root to display the actual calculated path of the config file instead of hardcoding a path.

I wasn't sure of the preferred approach to naming/error-handling, and it's been years since I've written any Go, so I tried to match the existing code as closely as possible. Feedback/changes absolutely welcome. 🙂

Checklist

  • My code follows the Contribution Guide
  • I have self-reviewed the changes being requested
  • I have made comments on pieces of code that may be difficult to understand for other editors
  • I have updated the documentation (if applicable)

@Xemdo
Copy link
Contributor

Xemdo commented Apr 5, 2022

This issue is actually beyond Mac OS.
Currently, if the $HOME/.twitch-cli/ folder exists, its used. However, if it doesn't exist, then instead it uses the result of os.UserConfigDir(), which is particularly different to each system:

  • Unix/Linux: $XDG_CONFIG_HOME (if exists), or $HOME/.config
  • Windows: %APPDATA%
  • Darwin/Mac OS: $HOME/Library/Application Support

(Its also worth noting that MinGW on Windows uses $HOME/.config)

Your contribution here is a step forward for sure, and it highlights an underlying problem that now needs fixing. Specifically, this seemingly (from what I can tell) undocumented change in config location.

I'll approve this and merge it. I appreciate you finding and fixing this!

@Xemdo Xemdo merged commit 237e563 into twitchdev:main Apr 5, 2022
@lleadbet
Copy link
Contributor

lleadbet commented Apr 5, 2022

@Xemdo - I can actually provide color there since it's relevant.

The change wasn't documented since it wasn't going to impact folks, nor are people usually modifying the db or config files manually. With that said, there was a specific request from the community per issue #33.

We were previously using a hardcoded user profile/.twitch-cli/ folder, which wasn't optimal for *nix systems, and the config directory should be respected nonetheless.

@Xemdo
Copy link
Contributor

Xemdo commented Apr 5, 2022

@lleadbet Ah, makes sense. With this PR then, it would show the user where it currently is rather than where it would be if they were a fresh install.

As you mentioned the config directory should definitely be respected, so I'll leave that as is 😃
Only thing that still gets me is the MINGW config, but if someone's using an emulated Unix environment I expect them to be ready for $HOME to be prioritized over %APPDATA%.

Thank you for the input on this!

@Arclite Arclite deleted the fix-config-path branch April 18, 2022 08:58
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.

3 participants