Skip to content

New merge command #27

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

Merged
merged 7 commits into from
Dec 18, 2020
Merged

New merge command #27

merged 7 commits into from
Dec 18, 2020

Conversation

lukasgierth
Copy link
Contributor

Hi,

i added a simple merge command which simply calls "git merge $argv".
I use git merge quite often and was frustrated that i always have to use "git merge" instead of just "merge".

This first entry right now is only like an alias basically. But the plan is that the functionality can be expanded later on (dry runs etc.). I did not come up with a keyboard shortcut (that makes sense) yet.

Greetings from Germany,
stay healthy everyone!

@joseluisq joseluisq self-assigned this Dec 16, 2020
@joseluisq joseluisq added the enhancement New feature or request label Dec 16, 2020
@joseluisq
Copy link
Owner

Hi, that's no bad idea to have the ease of typing during a merge via Fish auto completion.
However I would like to see some usage examples of that command (with no arguments and with them).

Copy link
Owner

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

Also I would like to see some updates on the README for this new command with some examples.

end

set -l len (count $argv)
set -l opts .
Copy link
Owner

Choose a reason for hiding this comment

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

Since the default argument looks like is a dot, a simple merge (git merge .) execution throws an error like fatal: . - not something we can merge.
If some argument is needed, I suggest to add some checks.

@lukasgierth
Copy link
Contributor Author

I added some checks (does the branch exist, is it not the already active one etc.). I also added the -a option (instead of the branch name) to abort a conflicted branch.
Right now it only checks if zero arguments are given. Should we better check that exactly one is give (so either a branch or the -a option)?
I also updated the README.

Some examples:
❯ merge sss
Local branch sss was not found. Not possible to merge.

❯ merge
Merge: No argument given, needs one parameter

❯ merge master
Branch master is the same as current branch. Nothing to do,

@joseluisq
Copy link
Owner

Right now it only checks if zero arguments are given. Should we better check that exactly one is give (so either a branch or the -a option)?

It's good idea since you need at least one arg.
However It will be even better if you make a distinguish between arguments like option flags (with dashes) and just branch names.
As a hint you can check the switch statement on tag command which is relative easy to implement.

@lukasgierth
Copy link
Contributor Author

If you look at my last commit, I already do use switch like you suggested

@joseluisq
Copy link
Owner

Yes, you already do it. Sorry I had not seen it.

BTW Check this bugfix for #28 which prevent an issue with the __gitnow_check_if_branch_exist.

@joseluisq
Copy link
Owner

I plan to merge this in few minutes after a last review.

@joseluisq
Copy link
Owner

When I try the merge command with option arguments (flags) then I get this:

merge --continue
# Local branch `` was not found. Not possible to merge.

I think you need to iterate over the $argv in order to know if you got a branch name param with options or without them.
The way to do it is implemented on move command.

gitnow/conf.d/gitnow.fish

Lines 256 to 281 in 708a95a

set -l v_branch
for v in $argv
switch $v
case -u --upstream
set v_upstream $v
case -n --no-apply-stash
set v_no_apply_stash $v
case -nu -un
set v_upstream "-u"
set v_no_apply_stash "-n"
case -h --help
echo "NAME"
echo " Gitnow: move - Switch from current branch to another but stashing uncommitted changes"
echo "EXAMPLES"
echo " move <branch to switch to>"
echo "OPTIONS:"
echo " -n --no-apply-stash Switch to a local branch but without applying current stash"
echo " -u --upstream Fetch a remote branch and switch to it applying current stash. It can be combined with --no-apply-stash"
echo " -h --help Show information about the options for this command"
return
case -\*
case '*'
set v_branch $v
end
end

Which means you will need another check when a branch is not provided.

gitnow/conf.d/gitnow.fish

Lines 283 to 289 in 708a95a

# No branch defined
if not test -n "$v_branch"
echo "Provide a valid branch name to switch to."
commandline -f repaint
return
end

@lukasgierth
Copy link
Contributor Author

Pushed a new commit which should satisfy your suggestions.

@joseluisq
Copy link
Owner

I have tested and works fine!

~> merge --continue
Continue the current merge
fatal: There is no merge in progress (MERGE_HEAD missing).
~> merge
Merge: No argument given, needs one parameter
~> merge -a
Abort the current merge
fatal: There is no merge to abort (MERGE_HEAD missing).
~> git merge --help
~> merge --continue --no-verify
Continue the current merge
fatal: There is no merge in progress (MERGE_HEAD missing).
~> merge --no-verify --continue
Continue the current merge
fatal: There is no merge in progress (MERGE_HEAD missing).
~> merge --no-verify
Provide a valid branch name to switch to

@joseluisq joseluisq changed the title Simple merge command New merge command Dec 17, 2020
Copy link
Owner

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

There is the last remaining change that should happen in order to merge this PR.

Copy link
Owner

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

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

Thanks!

@joseluisq joseluisq merged commit 10e064c into joseluisq:master Dec 18, 2020
@joseluisq joseluisq added this to the 2.6.0 milestone Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants