nushell.github.io icon indicating copy to clipboard operation
nushell.github.io copied to clipboard

Fix Keychain instructions in ssh_agent.md

Open justbispo opened this issue 1 year ago • 1 comments

Currently, the Keychain instructions in the cookbook don't work if you're trying to use GPG and SSH. The instructions assumes you're only using SSH.

With this change, it now removes the empty lines and uses the remaining ones, instead of only using the first 2.

Imo, ideally the page should also mention GPG. But my knowledge is superficial in this matter so I'll leave it to others.

justbispo avatar May 04 '24 17:05 justbispo

@amtoine Seems like you made the most recent changes to this file, iirc. Could you look over this PR and provide feedback or land it please?

fdncred avatar May 06 '24 11:05 fdncred

mm actually, i don't know nothing about keychain, i only wrote the SSH part above i think :confused: i'm using a slightly modified version of the Workaround section :yum:

amtoine avatar May 08 '24 07:05 amtoine

maybe if @justbispo can give a sample output of a keychain call, that'd be possible to see if the change is legit or not, otherwise i'm afraid i can't help much

amtoine avatar May 08 '24 07:05 amtoine

Sure, the command I have on my config is keychain --eval --quiet --noask -Q --agents gpg,ssh id_rsa id_ed25519 and the output is:

GPG_AGENT_INFO=/run/user/1000/gnupg/S.gpg-agent:1968:1; export GPG_AGENT_INFO;

SSH_AUTH_SOCK=/tmp/ssh-XXXXXXyd2dk5/agent.2138; export SSH_AUTH_SOCK;
SSH_AGENT_PID=2139; export SSH_AGENT_PID;


justbispo avatar May 08 '24 11:05 justbispo

That works too and it's much easier to understand than use the reduce. Also, can the call

load-env (
	...
	)

be replaced with

	...
	| load-env

to not use a different syntax of the first example of the page? It seems it can after testing it, but I'm still new to nushell so I'm just trying to be sure.

justbispo avatar May 08 '24 12:05 justbispo

@justbispo yeah,

{ FOO: "this is foo" } | load-env

and

load-env { FOO: "this is foo" }

yield the same results :ok_hand:

amtoine avatar May 10 '24 07:05 amtoine

@amtoine alright, thanks for your suggestion, just implemented it. Should be good to merge.

justbispo avatar May 10 '24 15:05 justbispo