Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

Jonathan Nieder writes: > Junio C Hamano wrote: >> Jonathan Nieder writes: > >>> Handles the nongit case in strbuf_check_branch_ref instead of >>> introducing a new check_branch_ref_format helper. >> >> I view that as a regression, actually. Don't we

Re: [PATCH] check-ref-format: require a repository for --branch

Junio C Hamano wrote: > Jonathan Nieder writes: >> And in that spirit, I think the patch you replied with aims to go in >> the right direction, by providing the core functionality when in a >> repository while avoiding breaking such a script outside of one >> (though I do not

Re: [PATCH] check-ref-format: require a repository for --branch

Junio C Hamano wrote: > Jonathan Nieder writes: >> And in that spirit, I think the patch you replied with aims to go in >> the right direction, by providing the core functionality when in a >> repository while avoiding breaking such a script outside of one >> (though I do not

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

Junio C Hamano wrote: > Jonathan Nieder writes: >> Handles the nongit case in strbuf_check_branch_ref instead of >> introducing a new check_branch_ref_format helper. > > I view that as a regression, actually. Don't we want a function > that does not require a strbuf when

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

Jonathan Nieder writes: > Handles the nongit case in strbuf_check_branch_ref instead of > introducing a new check_branch_ref_format helper. I view that as a regression, actually. Don't we want a function that does not require a strbuf when asking a simple question: "I have

[PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

Hi, Junio C Hamano wrote: > Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a > repository How about this? It is written to be more conservative than the patch I am replying to, but except for the commit message, it should be pretty much equivalent. [...] > ---

Re: [PATCH] check-ref-format: require a repository for --branch

Jonathan Nieder writes: > And in that spirit, I think the patch you replied with aims to go in > the right direction, by providing the core functionality when in a > repository while avoiding breaking such a script outside of one > (though I do not understand it fully yet).

Re: [PATCH] check-ref-format: require a repository for --branch

Junio C Hamano writes: >> I don't think there is any need to prepare it upon my 4d03f955, >> though. I'd think it could simply replace it. > > Yeah, it ended up that way, it seems. Still it needs a bit of doc > updates to balance the description. So here is what I have now

Re: [PATCH] check-ref-format: require a repository for --branch

Hi, Junio C Hamano wrote: > Things like @{-1} would not make any sense when the command is run > outside a repository, and the documentation is quite clear that it > is the primary reason why we added "--branch" option to the command, > i.e. > > With the `--branch` option, it expands the

Re: [PATCH] check-ref-format: require a repository for --branch

On Tue, Oct 17, 2017 at 11:40:17AM +0900, Junio C Hamano wrote: > Kevin Daudt writes: > > >> + setup_git_directory_gently(); > >> + > >> + if (!nongit) > >> + malformed = (strbuf_check_branch_ref(, arg) || > >> + !skip_prefix(sb.buf,

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King writes: > On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote: > >> > I like the state this puts us in, but there's one catch: we're >> > completely changing the meaning of "check-ref-format --branch", aren't >> > we? >> > >> > It is going from "this is how

Re: [PATCH] check-ref-format: require a repository for --branch

On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote: > > I like the state this puts us in, but there's one catch: we're > > completely changing the meaning of "check-ref-format --branch", aren't > > we? > > > > It is going from "this is how you resolve @{-1}" to "this is how you > >

Re: [PATCH] check-ref-format: require a repository for --branch

Kevin Daudt writes: >> +setup_git_directory_gently(); >> + >> +if (!nongit) >> +malformed = (strbuf_check_branch_ref(, arg) || >> + !skip_prefix(sb.buf, "refs/heads/", )); >> +else >> +malformed =

Re: [PATCH] check-ref-format: require a repository for --branch

On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > Junio C Hamano writes: > > [..] > > diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c > index 1e5f9835f0..4e62852089 100644 > --- a/builtin/check-ref-format.c > +++

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King writes: > On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > >> > So I think the right endgame in the longer term is: >> > ... >> >> Here is to illustrate what I mean in a patch form. It resurrects >> the gentle setup, and uses a purely textual format

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King wrote: > On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: >> Here is to illustrate what I mean in a patch form. It resurrects >> the gentle setup, and uses a purely textual format check when we are >> outside the repository, while bypassing the @{magic} interpolation >>

Re: [PATCH] check-ref-format: require a repository for --branch

On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > > So I think the right endgame in the longer term is: > > ... > > Here is to illustrate what I mean in a patch form. It resurrects > the gentle setup, and uses a purely textual format check when we are > outside the repository,

Re: [PATCH] check-ref-format: require a repository for --branch

On Mon, Oct 16, 2017 at 03:44:08PM +0900, Junio C Hamano wrote: > I threw this topic in stalled category, hoping that one or the other > opinion eventually turns out to be more prevalent, but it didn't > seem to have happened X-<. I think it's sufficiently obscure that nobody really cares. I

Re: [PATCH] check-ref-format: require a repository for --branch

Junio C Hamano writes: > Having said that, there may still be a use case where a Porcelain > script wants a way to see if a $name it has is appropriate as a > branch name before it has a repository (e.g. a wrapper to "git > clone" that wants to verify the name it is going to

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King writes: > On Mon, Jul 17, 2017 at 10:27:09AM -0700, Jonathan Nieder wrote: >> ... >> I don't think it's right. Today I can do >> >> $ cd /tmp >> $ git check-ref-format --branch master >> master >> >> You might wonder why I'd ever do such a thing. But

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King writes: > I _do_ think it's a misfeature to put it in check-ref-format. It should > be part of rev-parse. Which admittedly is a kitchen sink, but this kind > of resolving is one of the main things it should be doing. And in fact > you can already do: > > git rev-parse

Re: [PATCH] check-ref-format: require a repository for --branch

On Thu, Aug 17, 2017 at 02:30:53PM -0700, Junio C Hamano wrote: > > I'm not sure I buy that. What does "turning it into a branch name" even > > mean when you are not in a repository? Clearly @{-1} must fail. And > > everything else is just going to output the exact input you provided. > > This

Re: [PATCH] check-ref-format: require a repository for --branch

Jeff King writes: > On Mon, Jul 17, 2017 at 10:27:09AM -0700, Jonathan Nieder wrote: > >> > --- a/t/t1402-check-ref-format.sh >> > +++ b/t/t1402-check-ref-format.sh >> > @@ -161,6 +161,10 @@ test_expect_success 'check-ref-format --branch from >> > subdir' ' >> >test

Re: [PATCH] check-ref-format: require a repository for --branch

On Mon, Jul 17, 2017 at 11:18:43PM +0200, Marko Kungla wrote: > I guess that should only be about that it should not hit a (BUG). > In my case in the example I gave I scan trough the directories to > check repository status one of the tasks make use of check-ref-format. > Since it may hit

Re: [PATCH] check-ref-format: require a repository for --branch

On Mon, Jul 17, 2017 at 10:27:09AM -0700, Jonathan Nieder wrote: > > --- a/t/t1402-check-ref-format.sh > > +++ b/t/t1402-check-ref-format.sh > > @@ -161,6 +161,10 @@ test_expect_success 'check-ref-format --branch from > > subdir' ' > > test "$refname" = "$sha1" > > ' > > > >

Re: [PATCH] check-ref-format: require a repository for --branch

I guess that should only be about that it should not hit a (BUG). In my case in the example I gave I scan trough the directories to check repository status one of the tasks make use of check-ref-format. Since it may hit directory which is not a git repository it should not expose error (BUG)

Re: [PATCH] check-ref-format: require a repository for --branch

Hi, Jeff King wrote: > On Fri, Jul 14, 2017 at 02:03:13PM -0400, Jeff King wrote: >> So I think the patch below is probably the right direction. > > And here it is with a real commit message, if this is what we want to > do. [...] > --- a/t/t1402-check-ref-format.sh > +++

[PATCH] check-ref-format: require a repository for --branch

to fit better with "rev-parse" (which yes, is a kitchen sink, but it does all sorts of similar resolution operations). I think "git rev-parse --symbolic-full-name @{-1}" is basically the same thing (modulo the refs/heads/ shortening). -- >8 -- Subject: [PATCH] check-ref-format: req