Another fix for #557 #565

wants to merge 56 commits into


None yet
5 participants

dudebout commented Feb 22, 2013

The part specifying the destitnation as origin/master was not specific enough. It would create a local branch named origin/master instead finding refs/remotes/origin/master. I misunderstood what was said in man git rev-parse.

sigma and others added some commits Jun 16, 2012

magit-push: only push to branch.b.merge when pushing to branch.b.remote
The branch specified by the former is only meaningful when pushing to
the remote specified by the latter.  When pushing to another remote
and the remote branch is not specified explicitly then pusing to the
remote branch with the same name as the local branch being pushed is
much more reasonable than pushing to branch.b.merge which could be
anything.  This is also how "git push <remote> <branch>" behaves.
always respect value of magit-set-upstream-on-push
Previously both branch.<name>.remote and branch.<name>.merge where set
under certain conditions ignoring the value of magit-set-upstream-on-push:
when both branch.<name>.remote and remote.origin.url are unset
branch.<name>.remote was always set.  When additionally no prefix argument
is used branch.<name>.merge was also set ignoring the value of
magit-builtin-completing-read: handle REQUIRE-MATCH safely
completing-read does allow the user to exit with null input, leaving
it to the caller to handle that case.  Possible options would be to
use some default value or raise an error.

Since magit-builtin-completing-read is a convenience wrapper for
completing-read it seems reasonable to do the latter here so that not
every caller of magit-completing-read has to do this on it's own.

A similar change is not required for magit-iswitchb-completing-read
and magit-ido-completing-read; they already don't allow the user to
exit with a null value.
magit-read-remote: require that an existing remote is selected
All current uses of magit-read-remote actually require a remote to
be returned as the behaviour of nil being returned is not defined.
E.g. magit-push would try to push to a remote with the name "nil".

It would be better to give magit-read-remote a REQUIRE-MATCH argument
but since all callers expect a match this is not strictly unnecessary
at the moment.  Also some other magit-read-* functions could benefit
from a similar change.
magit-push: ensure we are pushing to a branch
Do this by always using the full ref instead of just the name for
the remote branch.  Without this we could end pushing to some other
non-branch ref if no branch by the specified name exists remotely
but some other ref by that name does.
magit-push: set branch.<name>.merge correctly
The backward compatibility hack that ensured that branch.<name>.merge
was set even when using an old version of git used the wrong local
variable.  In many cases that variable contains the correct value but
this does not have to be the case.  More specifically it always used
the same ref as the ref of the local branch.  When the user selected
another branch name that obviously is not correct.  So this kludge for
older git versions could actually harm users of recent versions.
magit-push: do not use magit-get-remote to get the branch-remote
The meaning of "branch-remote" is "the remote configured for branch"
but if no remote is configured magit-get-remote might return "origin"
instead.  This is wrong here because we later use the value of
branch-remote to determine if we we should push to branch.<name>.merge
or just <name>.

So this ensures that we do the right thing when branch.<name>.merge is
set but branch.<name>.remote is not.
magit-push: update doc-string to reflect recent changes
Actually this new doc-string - for the bigger part - would have been
correct even before these changes since most of them are actually
bugfixes.  So the new doc-string wouldn't have been more wrong than
the old one :-)

No longer say that in certain cases something "hairy" is done when
that behaviour is well defined in git-push(1) and also reasonable.
Instead explicitly say what is done (pushing to the remote branch
with the same name as the local branch) and under what conditions.

I think the new doc-string is much easier to understand.  For better
readability I removed the instructions how prefix arguments are used;
the user has to get that knowledge elsewhere.  Not every command that
supports prefix arguments documents that basics of their use; why
should this one be different?
magit-push: set branch.<name>.merge when -u is in magit-custom-options
When --set-upstream is set through magit-key-mode use the same
backward compatibility hack to ensure branch.<name>.merge is set
even when using an old git version as is used when --set-upstream
is used due to the value of the magit-set-upstream-on-push option.
magit-run*: only magit-mode buffers need to be refreshed
Refreshing means that point moves to the beginning of the line which
is not desirable for non-magit-mode buffers.  This fixes issue #441.
magit-key-mode: don't delete other windows to show menu
Signed-off-by: Ramkumar Ramachandra <>

sigma and others added some commits Feb 6, 2013

magit-prefix-p: improve doc-string
The old doc-string wasn't very precise, e.g. it left out that `equal'
is used for comparison.

Also rename arguments;  It just doesn't feel right to say.

  PREFIX is a prefix ... if ...

The hole point of this function is to check whether the first list is
a prefix of the second.  It might very well not be, so let's just call
it a list.
refactor magit-section-case and related functions
`magit-section-case' used to return t if a clause succeeds but the
value of it's body is nil.  This was due to internal needs; it allowed
`run-hook-with-args-until-sucess' to detect that a clause and therefor
the hook function succeeded.  However, while it was documented in the
doc-string, callers should not have to deal with this.

* `magit-section-case' and `magit-section-action' always return
  the value of the BODY now.  In other words they can return nil.
* `magit-section-case' doesn't need to know about OPCODE.
  Running the OPCODE hook is done in `magit-section-action' now.
* `magit-add-section' wrap each BODY so that `magit-section-action'
  can detect whether a CLAUSE succeeded.  Previously that was done
  in `magit-section-case' for *any* CLAUSE.
* Disallow a t/otherwise CLAUSE in `magit-section-action' as that
  would prevent the OPCODE hook from running.
* Replace uses of `magit-section-action' without OPCODE to use
  `magit-section-case' instead.

@dudebout dudebout closed this Feb 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment