#151 ✓resolved
Stoney

Create branch fails silently

Reported by Stoney | May 15th, 2009 @ 07:58 PM

If I click on the Create Branch button, and type in a branch name containing a space, the branch is not created, and no error is reported.

Comments and changes to this ticket

  • Charles O'Rourke
  • Johannes Gilger

    Johannes Gilger May 27th, 2009 @ 06:06 PM

    • State changed from “new” to “small”

    Cool thing. Two things I'd like to see before I think we could incorporate this patch:

    1. A return-code != 0 doesn't just mean "invalid branch name" but could also mean "branch already exists". Unfortunately the return-code for both operations is 128, so you'll have to check the output in case the return-code is !=0
    2. The diff of the XIB-file reveals a file "public.png", but your commit does not. Since the XIB-files have become quite readable IMHO, we should also just stage the changes we intended to.
  • Johannes Gilger

    Johannes Gilger May 27th, 2009 @ 06:16 PM

    Wow, that comment was just unnecessary. I just read your code more closely and saw that you're using check-ref-format. Ok. But what you could also do (while you're at it) is to check the return-value of update-ref and use your new error-message field to indicate failure (because of already existing refs). This would happen after line 206 in your patch.

  • Johannes Gilger

    Johannes Gilger May 27th, 2009 @ 06:27 PM

    Again, sorry, I didn't see you head two commits which address this problem. This calls for speedy upstream inclusion.

  • Pieter de Bie

    Pieter de Bie May 27th, 2009 @ 06:32 PM

    While I agree that we need to check for valid names and existing refs,
    I don't like this approach. This way we need three calls to git before
    creating a ref.

    The check-ref-format call might be OK, but for what it does (as is
    described in the manpage), we might just as well recreate it
    ourselves. The existing branch thing should be handled as I mentioned
    in the other ticket.

  • Charles O'Rourke

    Charles O'Rourke May 28th, 2009 @ 02:59 AM

    I only count one call to git before creating the ref - the call to check-ref-format. What advantage is there to recreating it when check-ref-format does it for us? I'd think using that means that if they change the rules down the line, we inherit the new rules by virtue of using the same function that git's tools are using.

  • Charles O'Rourke

    Charles O'Rourke May 28th, 2009 @ 03:20 AM

    Oh, I see, you're referring to the call to show-ref as the second call. OK - here is another commit (merging both tickets #151 and 155) and using update-ref with the 40 "0"'s rather than show-ref, along with cleaning up the commit.

    http://github.com/shadesofblue/gitx/commit/81841e854805befcb10d510f...

  • Pieter de Bie

    Pieter de Bie May 28th, 2009 @ 02:23 PM

    • State changed from “small” to “resolved”

    Thanks, this looks good and I've cherry-picked it into my stable
    branch. This is probably important enough for the stable release.

    One thing I'd like to see in a future release is that the sheet(s) get
    their own .xib and controller. The logic is starting to become complex
    enough to be allow them to be separated. That'd also allow us to
    create a new branch from the commit view, for example.

    [state:resolved]

  • Ashwin kumar

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

GitX is the nice-looking gitk clone for OS X

Referenced by

Pages