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

feat(git-fork): add flags comprehension #1053

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mjholub
Copy link

@mjholub mjholub commented Jun 22, 2023

Initially I just wanted to add support for directly forking the current repo (say you clone something, notice something you want/need to change, and you don't want to use git-fork $(git remote get-url origin), so I added the -c and --current flags for convenience, along with the help, token and target dir flags (because I'd rather keep my forks in ~/projects/forks, than the subdirectory of the upstream repo, which I usually keep in ~/src)

@@ -0,0 +1,184 @@
<!DOCTYPE html>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move it to git-fork.html?

2. clones the repo into the current dir
3. adds the original repo as a remote called `upstream`
1. Forks the repo on GitHub.
2. Clones the repo into the current directory or the one specified with `d` or `--target-dir`.
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
2. Clones the repo into the current directory or the one specified with `d` or `--target-dir`.
2. Clones the repo into the current directory or the one specified with `-d` or `--target-dir`.

local merged_opts_str=""
merged_opts_str+="$(printf "%s " "${s_opts[@]}")"
merged_opts_str+="$(printf "%s " "${l_opts[@]}")"
_git_changelog() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you avoid changing the indent of other commands' functions? Thanks!

if [[ "${cur}" == -* ]]; then
COMPREPLY=($(compgen -W "${opts}" -- ${cur}))
return 0
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can use fi here or move the common return 0 out?

for completion in $completions
echo $completion
end
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems the execution is already return in every branch?

local clone_dir="${target_dir:-$project}"
git clone "${remote_prefix}${user}/${project}.git" "$clone_dir"
# Add reference to the original fork for merging upstream changes
cd "$clone_dir" || exit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we exit with non-zero?

[ -n "$user" ] || abort "GitHub username is required"

# Prompt for personal access token if not provided
if [ -z "$github_personal_access_token" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems we no longer get the token from git config anymore

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.

2 participants