-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add brew standard installation path for gpg common paths #145
base: master
Are you sure you want to change the base?
Add brew standard installation path for gpg common paths #145
Conversation
Thanks! This was brought up before, I'm still not sold on including such custom paths in the default fallback list; homebrew is although popular not the only package manager for macOS, and even homebrew has changed the folder they use when M1 was released. I don't think we should keep up with all the package managers in the world, this is really meant more like a fallback list. You have correctly identified that the actual issue is that If I remember correctly, one way to fix this on macOS is to run Give it a go! It's a useful thing to do potentially even for other apps 🙂 If you can make it work, I'd appreciate a PR for a new FAQ entry for this topic from someone who recently went through the steps and confirmed which exact steps solve the issue 😉 |
I totally understand your point, following all package manager default installation conventions could be a not so fun approach. On the other hand, the installation process is already a bit opaque and really not user friendly. I found out what was happening but it was really not easy, I've try hard to make it work. The fact that used to work on my previous mac is even more confusing for me ^^' I would argue that we should either support most popular package manager as default path, or not provide a default path at all and always require this setting. Because today, it's confusing and it's difficult to know what is happening. Another approach could be de show a very clear message that indicate that the configuration is broken and show a clear message about how to fix it. Because today, the UI will just show a JSON blob error in an ephemeral alert (which is not displayed entirely because the window is generaly too small). We should check upfront if the config is working, because letting user see the passwords list and failing only once you try to retreive it is not a very happy path ^^' |
Couldn't we spawn a shell and try to get pgp path from the shell session instead of relying on GUI env ? |
It would probably work in even less cases, not only because you need to guess the shell (not just launching
This is certainly a good idea to improve the error handling, the error json blob is something that was meant to be improved since the beginning, but never ended up being acted upon 😅 I can totally relate to everything you say about the experience being confusing, difficult and not user-friendly, this deserves to be improved! This is one reason why I like your idea about better error message, it would help everyone, no matter which funky unpopular distro or package manager they used, everyone deserves to have a good experience to figure out what is wrong and how to make the extension work. One other reason that again speaks in favour of error messages is that it can be done purely on frontend - the distribution of native component is a painful process, so it was designed with the goal of being as thin of a layer as possible, something that almost never has to be updated - and maintaining lists of package managers would go against that core principle as well. Do you have the energy or enthusiasm in you to have a look and propose how a better error message might look like? No need to solve everything at once, just getting this one specific error about missing "gpg" presented in a better way would be a very good start? |
I see :/ I wonder how they found a way to make it work in tools like VSCode ^^' I see your point about requiring as less frequent updates as possible. You totally don't imagine myself manually update the native app companion :-) So yeah, better UI with better errors display is probably the way to go with this. I was imagining a "first experience" screen, with a series of checks, like:
Not sure where or when this screen could be shown by the way. Perhaps we can keep some state in settings to know if this first checks have been passed or not, and display it instead of the actual password list if not ? I also remember that extensions can open a page right after its installation. Perhaps this could also be a good place to show this "setup" screen :-) |
Let's do it even simpler, we don't need to keep a state, or distinguish first launch; the extension talks to native host and fetches settings on every single interaction, so for example if it couldn't talk to native host, we should show a good and descriptive error about that (instead of the currently generic "Error: Error: ..."), and similarly if gpg was not found, a different descriptive error could be shown. What makes it even better is that native host already sends descriptive errors, so it's purely the frontend part that needs to be improved 🙂 If you want to play around, I believe this place might be a good starting point, to see what kind of data there is, and then how this variable is currently getting used. |
On macOS, we generally use
brew
to installgpg
. This PR aims to add the brew installedgpg
binary path to the common paths to look up for.In my case, for some unknown reason,
gpg
was not resolved while it is actually part of the$PATH
variable.