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

Fix Keychain instructions in ssh_agent.md #1388

Merged
merged 2 commits into from
May 11, 2024
Merged

Conversation

justbispo
Copy link
Contributor

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.

@fdncred
Copy link
Collaborator

fdncred commented May 6, 2024

@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?

@amtoine
Copy link
Member

amtoine commented May 8, 2024

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

@amtoine
Copy link
Member

amtoine commented May 8, 2024

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

@justbispo
Copy link
Contributor Author

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;


Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @justbispo 🙏

with

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;

and without considering the load-env call, this PR goes from

Error: nu::shell::column_not_found

  × Cannot find column
   ╭─[entry #4:9:54]
 8 │     | rename name value
 9 │     | reduce -f {} {|it, acc| $acc | upsert $it.name $it.value }
   ·                                                      ─┬─ ──┬──
   ·                                                       │    ╰── cannot find column 'value'
   ·                                                       ╰── value originates here
   ╰────

to

──────────────┬───────────────────────────────────────
GPG_AGENT_INFO│/run/user/1000/gnupg/S.gpg-agent:1968:1
SSH_AUTH_SOCK │/tmp/ssh-XXXXXXyd2dk5/agent.2138
SSH_AGENT_PID │2139
──────────────┴───────────────────────────────────────

what about the following though?

keychain ...
    | lines
    | where not ($it | is-empty)
    | parse "{k}={v}; export {k2};"
    | select k v
    | transpose --header-row
    | into record

looks much simpler ot me :)

@justbispo
Copy link
Contributor Author

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.

@amtoine
Copy link
Member

amtoine commented May 10, 2024

@justbispo
yeah,

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

and

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

yield the same results 👌

@justbispo
Copy link
Contributor Author

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

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks pretty good to me, let's see how it goes 🙏

@amtoine amtoine merged commit 325cb6b into nushell:main May 11, 2024
2 checks passed
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.

None yet

3 participants