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

Auto Git Config Script Added and Tab_Data TOML Updated. #448

Closed
wants to merge 10 commits into from

Conversation

fam007e
Copy link

@fam007e fam007e commented Sep 17, 2024

Type of Change

  • New feature

Description

This PR introduces a script that automates the configuration of Git for users, allowing them to:

  • Set up their GitHub email.
  • Generate an SSH key of their chosen type (Ed25519 or RSA).
    Additionally, it includes consequential changes to the tab_data.toml file for proper integration.

Testing

  • Verified GitHub email input and SSH key generation (Ed25519 and RSA).
  • Tested SSH connection to GitHub to ensure proper setup.

Impact

This change simplifies the Git setup process, ensuring consistency across environments and making it easier for users to configure their Git and GitHub SSH keys.

Issue related to PR

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no errors/warnings/merge conflicts.

Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

You should move this to utils/

tabs/system-setup/tab_data.toml Outdated Show resolved Hide resolved
tabs/system-setup/git-auto-conf-cli.sh Outdated Show resolved Hide resolved
tabs/system-setup/git-auto-conf-cli.sh Outdated Show resolved Hide resolved
Comment on lines 68 to 81
# Prompt user to confirm they've added the key to GitHub
while true; do
printf "Have you pasted your SSH public key into your GitHub account? (y/n): "
read yn
case "$yn" in
[Yy]* ) echo "Proceeding..."; break ;;
[Nn]* ) echo "Please paste your SSH public key into GitHub and try again."; exit ;;
* ) echo "Please answer yes (y) or no (n)." ;;
esac
done

# Test the SSH connection with GitHub
ssh -T [email protected]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if this GitHub part is neccessary. @nnyyxxxx ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks useless

Copy link
Contributor

Choose a reason for hiding this comment

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

and invasive

Copy link
Contributor

Choose a reason for hiding this comment

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

the users dont know if the information they're putting into linutil is being tracked or not without looking at the source code so this is bad @fam007e

Copy link
Author

Choose a reason for hiding this comment

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

But as far as I know linutil is binary running in user's specific environment so text being in user's clipboard is NOT supposed be an issue here.
Since you have raised an issue I will change it anyway unlike what my opinion is on this matter.

@adamperkowski
Copy link
Collaborator

adamperkowski commented Sep 17, 2024

Your script should also check if git is installed and actually set up commit signing (and set up git overall) with the generated key.

@adamperkowski
Copy link
Collaborator

adamperkowski commented Sep 17, 2024

@fam007e Can you draft this PR for now (and the other ones as well)? Chris is going to be streaming soon and with almost 60 open PRs let's just not waste his time before this is done.

@nnyyxxxx
Copy link
Contributor

Yeah i'm going to just say it now, THIS PR is a total mess; and a joke, you're using functions in common-script that shouldn't be used for checking deps like curl or git, and on top of that you're asking the user if they want to test their GIT SSH key which is sooo invasive. Please just close this.

else
echo "Skipping passphrase setup."
fi
echo "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]"
printf "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]"

You CANNOT use new lines in echo without it being non-posix, use printf

@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

Yeah i'm going to just say it now, THIS PR is a total mess; and a joke, you're using functions in common-script that shouldn't be used for checking deps like curl or git, and on top of that you're asking the user if they want to test their GIT SSH key which is sooo invasive. Please just close this.

But @adamperkowski told me to add deps check for git... 🙃

…oved to utils dir as a concsequence affected tabs TOML is edited
@nnyyxxxx
Copy link
Contributor

Yeah i'm going to just say it now, THIS PR is a total mess; and a joke, you're using functions in common-script that shouldn't be used for checking deps like curl or git, and on top of that you're asking the user if they want to test their GIT SSH key which is sooo invasive. Please just close this.

But @adamperkowski told me to add deps check for git... 🙃

And I'm telling you that your implementation is wrong 👎


# Function to prompt for GitHub configuration
setup_git_config() {
checkCommandRequirements "git"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this implementation is completely WRONG, you clearly do not know what you're doing, using "checkcommandrequirements" from common script will exit the script if git is not found which is BAD and should not be used in this SCENARIO 👎

@nnyyxxxx
Copy link
Contributor

Draft these PRs they are not ready in the slightest.

@@ -51,3 +51,4 @@ script = "3-global-theme.sh"
[[data]]
name = "Remove Snaps"
script = "4-remove-snaps.sh"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Remove this line...

Copy link
Contributor

Choose a reason for hiding this comment

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

@fam007e ONCE AGAIN, this extra line has not been removed, please remove it


# Main execution
checkEnv
checkCommandRequirements "git"
Copy link
Contributor

Choose a reason for hiding this comment

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

NO. This implementation is STILL wrong, what do you not understand? I explained it to you in a previous review; Exiting the script if git is not found is NOT good. Do not implement it like this...


# Main execution
checkEnv
checkCommandRequirements "git"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to do what this PR does: #436


# Main execution
checkEnv
checkCommandRequirements "git"
Copy link
Contributor

Choose a reason for hiding this comment

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

@fam007e The other review on this issue is NOT resolved, I don't understand what you do not understand this implementation is incorrect and will exit the script upon git not being found, this is not what we want. Seriously dude.. We have tried explaining to you many times over.

Comment on lines 13 to 16
printf "Choose your SSH key type:"
printf "1. Ed25519 (recommended)"
printf "2. RSA (legacy)"
printf "Enter your choice (1 or 2): "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not like that. This is how it looks with printfs:
Choose your SSH key type:1. Ed25519 (recommended)2. RSA (legacy)Enter your choice (1 or 2):

Suggested change
printf "Choose your SSH key type:"
printf "1. Ed25519 (recommended)"
printf "2. RSA (legacy)"
printf "Enter your choice (1 or 2): "
echo "Choose your SSH key type:"
echo "1. Ed25519 (recommended)"
echo "2. RSA (legacy)"
printf "Enter your choice (1 or 2): "

1) key_algo="ed25519" ;;
2) key_algo="rsa" ;;
*)
printf "Invalid choice. Exiting."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
printf "Invalid choice. Exiting."
echo "Invalid choice. Exiting."

Comment on lines 40 to 49
printf "Do you want to use a passphrase? (y/n): "
read use_passphrase

# If user opts for a passphrase, add key to SSH agent
if [ "$use_passphrase" = "y" ]; then
ssh-add -l >/dev/null 2>&1 || eval "$(ssh-agent -s)"
ssh-add "$ssh_key_path"
else
printf "Skipping passphrase setup."
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
printf "Do you want to use a passphrase? (y/n): "
read use_passphrase
# If user opts for a passphrase, add key to SSH agent
if [ "$use_passphrase" = "y" ]; then
ssh-add -l >/dev/null 2>&1 || eval "$(ssh-agent -s)"
ssh-add "$ssh_key_path"
else
printf "Skipping passphrase setup."
fi
printf "Do you want to use a passphrase? (Y/n): "
read use_passphrase
case "$use_passphrase" in
n|N)
echo "Skipping passphrase setup."
;;
*)
ssh-add -l >/dev/null 2>&1 || eval "$(ssh-agent -s)"
ssh-add "$ssh_key_path"
;;
esac

else
printf "Skipping passphrase setup."
fi
printf "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
printf "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]"
printf "SSH key generation and setup completed.\nPlease copy the key from your ssh dir and paste it to your corresponding account ssh key add section of your github settings page.\nThen run this command to verify ssh connection:\nssh -T [email protected]\n"

@adamperkowski
Copy link
Collaborator

Draft this until everything is done.

@fam007e
Copy link
Author

fam007e commented Sep 17, 2024

Draft this until everything is done.

Plz check the recent commit. Hopefully I did not mess this up.

@nnyyxxxx
Copy link
Contributor

Draft this until everything is done.

Plz check the recent commit. Hopefully I did not mess this up.

Yeah.. you're creating more problems and it's being implemented incorrectly.

@nnyyxxxx
Copy link
Contributor

@fam007e Is this a joke?
image
Please close this PR, you're making it worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants