RE: [PATCH v3 0/6] Force pipes to flush immediately on NonStop platform

2018-01-19 Thread Randall S. Becker
> -Original Message-
> From: randall.s.bec...@rogers.com [mailto:randall.s.bec...@rogers.com]
> Sent: January 19, 2018 6:00 PM
> To: git@vger.kernel.org
> Cc: Randall S. Becker 
> Subject: [PATCH v3 0/6] Force pipes to flush immediately on NonStop
> platform
> 
> From: "Randall S. Becker" 
> 
> * wrapper.c: called setbuf(stream,NULL) to force pipe flushes not
>   enabled by default on the NonStop platform. This applies only
>   to the NonStop platform guarded by #ifdef __TANDEM.
> 
> Signed-off-by: Randall S. Becker 
> ---
>  wrapper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/wrapper.c b/wrapper.c
> index d20356a77..671cbb4b4 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)
>   FILE *stream = fdopen(fd, mode);
>   if (stream == NULL)
>   die_errno("Out of memory? fdopen failed");
> +#ifdef __TANDEM
> + setbuf(stream,NULL);
> +#endif
>   return stream;
>  }
> 
> --
> 2.16.0.31.gf1a482c

This is a replacement for the v2 patch based on Stefan's suggestions.
Cheers,
Randall



Re: [PATCH v3 0/6] Renames in git-status "changed not staged" section

2018-01-02 Thread Jeff Hostetler



On 12/27/2017 5:18 AM, Nguyễn Thái Ngọc Duy wrote:

v3 more or less goes back to v1 after my discussion with Igor about
porcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The
meat is still in 6/6, now with some more updates in git-status.txt and
to address the comment from Torsten.

Nguyễn Thái Ngọc Duy (6):
   t2203: test status output with porcelain v2 format
   Use DIFF_DETECT_RENAME for detect_rename assignments
   wt-status.c: coding style fix
   wt-status.c: catch unhandled diff status codes
   wt-status.c: rename rename-related fields in wt_status_change_data
   wt-status.c: handle worktree renames

  Documentation/git-status.txt | 23 ++--
  builtin/commit.c |  2 +-
  diff.c   |  2 +-
  t/t2203-add-intent.sh| 72 ++
  wt-status.c  | 83 
  wt-status.h  |  5 +--
  6 files changed, 143 insertions(+), 44 deletions(-)




Signed-off-by: Jeff Hostetler 



Re: [PATCH v3 0/6] Renames in git-status "changed not staged" section

2017-12-27 Thread Igor Djordjevic
Hi Duy,

On 27/12/2017 11:18, Nguyễn Thái Ngọc Duy wrote:
> 
> v3 more or less goes back to v1 after my discussion with Igor about
> porcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The
> meat is still in 6/6, now with some more updates in git-status.txt and
> to address the comment from Torsten.

Albeit a tiny concern expressed in that last e-mail[1], this now 
seems fine, and a few tests I did came back as expected. Thanks!

Regards, Buga

[1] 
https://public-inbox.org/git/CACsJy8A=jz9laum50gvjnt5gtdiyymymupbsrjfo4lmkvqs...@mail.gmail.com/T/#m18b4e2cb2b7685fcc9650f3fb71b2191ef74cbe1


Re: [PATCH v3 0/6] Partial clone part 1: object filtering

2017-11-07 Thread Junio C Hamano
Jeff Hostetler  writes:

> From: Jeff Hostetler 
>
> Here is V3 of the list-object filtering.  This addresses the
> comments on the mailing list for the V2 series as well as the
> various TODO items I left in the code.  I also documented some
> of the bit flags and fields that I added.
>
> In the blob size filter, I removed the ".git*" pattern matching
> for special files.  I don't think we need it any more and it
> simplifies the code.  This patch series does not support
> traverse_bitmap_commit_list() and the --use-bitmap-index feature
> in rev-list, but by removing the ".git*" pattern matching now
> we should be able allow filtering and bitmaps to be used
> together in a future effort.  (That is beyond the scope of
> the current partial-clone effort.)
>
> With this patch series, I think part 1 is complete unless there
> are further comments or questions.

Thanks.  

I tried to rebase the other two on top and queued them in 'pu'.


Re: [PATCH v3 0/6] config.h

2017-06-15 Thread Brandon Williams
On 06/15, Brandon Williams wrote:
> On 06/15, Junio C Hamano wrote:
> > Brandon Williams  writes:
> > 
> > > On 06/15, Junio C Hamano wrote:
> > >
> > >> ... so please eyeball the resulting 12 patches carefully when
> > >> they are pushed out.

I just took a look at what you pushed out and they look good to me.

-- 
Brandon Williams


Re: [PATCH v3 0/6] config.h

2017-06-15 Thread Brandon Williams
On 06/15, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > On 06/15, Junio C Hamano wrote:
> >
> >> ... so please eyeball the resulting 12 patches carefully when
> >> they are pushed out.
> >
> > Ugh, I'm terribly sorry.  Completely my bad as I didn't consider what
> > you would need to do on your end.  When I built my patches on top of his
> > I naively just applied his v4 to what ever the current origin/master was
> > at that point in time.  I'll be sure to be more careful with this
> > next time.
> 
> It's no big deal (otherwise I would have insisted you to rebase so
> that the end result can be merged to 'maint', instead of doing it
> myself).
> 
> But quite honestly, I do not understand why you rebased this on top
> of the alias thing.  Help me make sure that I correctly undertand
> what these two topics want to do:
> 
>  - The primary point of js/alias-early-config is to fix reading of
>pager config from a wrong place when alias expansion is involved,
>and its solution has a nice property that it simplifies the alias
>lookup and avoids the unpleasant save/restore-env dance by
>switching to use the early-config mechanism.
> 
>  - Unfortunately, the early-config mechanism is broken with respect
>to multi-worktree configuration because it does not pay attention
>to the common-dir stuff.
> 
>  - The primary objective of bw/config-h was to separate out the
>configuration related things out of the kitchen-sink cache.h, but
>as a nice side effect, it also fixes the early-config mechanism.
> 
> Assuming that the above reading of mine of these two topics are
> correct, we can conclude that even if we merge js/alias-early-config
> that forks from v2.13.0 to 'maint', the result by itself would
> regress the use of alias in multi-worktree configuration.  For it to
> be useful, it must be merged after bw/config-h gets merged.
> 
> So it looks to me that it would make more sense to build bw/config-h
> on v2.13.0 and then base js/alias-early-config on top of the result.
> If Dscho is too busy to rebase and you are volunteering to help,
> perhaps the right way to help would be for you to do that rebasing,
> not rebase bw/config-h on top of js/alias-early-config, which is
> backwards and does not buy us very much.  Of course, we could make
> the result of such rebasing into a single topic, but even in that
> case, the order of changes feel backwards if bw/config-h comes
> later.
> 
> Anyway, I think I have to tend to many more patches before I can
> push out today's integration result, so ...
> 
> 

Yeah I see that the order is probably backwards.  I think I just heard
Dscho say "I'm too busy" and went along with just rebasing on top of his
series.  If you do end up feeling like this should be done differently
(like if another reroll is needed etc) then I wouldn't mind taking
ownership and making the order more sane.

-- 
Brandon Williams


Re: [PATCH v3 0/6] config.h

2017-06-15 Thread Junio C Hamano
Brandon Williams  writes:

> On 06/15, Junio C Hamano wrote:
>
>> ... so please eyeball the resulting 12 patches carefully when
>> they are pushed out.
>
> Ugh, I'm terribly sorry.  Completely my bad as I didn't consider what
> you would need to do on your end.  When I built my patches on top of his
> I naively just applied his v4 to what ever the current origin/master was
> at that point in time.  I'll be sure to be more careful with this
> next time.

It's no big deal (otherwise I would have insisted you to rebase so
that the end result can be merged to 'maint', instead of doing it
myself).

But quite honestly, I do not understand why you rebased this on top
of the alias thing.  Help me make sure that I correctly undertand
what these two topics want to do:

 - The primary point of js/alias-early-config is to fix reading of
   pager config from a wrong place when alias expansion is involved,
   and its solution has a nice property that it simplifies the alias
   lookup and avoids the unpleasant save/restore-env dance by
   switching to use the early-config mechanism.

 - Unfortunately, the early-config mechanism is broken with respect
   to multi-worktree configuration because it does not pay attention
   to the common-dir stuff.

 - The primary objective of bw/config-h was to separate out the
   configuration related things out of the kitchen-sink cache.h, but
   as a nice side effect, it also fixes the early-config mechanism.

Assuming that the above reading of mine of these two topics are
correct, we can conclude that even if we merge js/alias-early-config
that forks from v2.13.0 to 'maint', the result by itself would
regress the use of alias in multi-worktree configuration.  For it to
be useful, it must be merged after bw/config-h gets merged.

So it looks to me that it would make more sense to build bw/config-h
on v2.13.0 and then base js/alias-early-config on top of the result.
If Dscho is too busy to rebase and you are volunteering to help,
perhaps the right way to help would be for you to do that rebasing,
not rebase bw/config-h on top of js/alias-early-config, which is
backwards and does not buy us very much.  Of course, we could make
the result of such rebasing into a single topic, but even in that
case, the order of changes feel backwards if bw/config-h comes
later.

Anyway, I think I have to tend to many more patches before I can
push out today's integration result, so ...




Re: [PATCH v3 0/6] config.h

2017-06-15 Thread Brandon Williams
On 06/15, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > Changes in v3:
> >
> > * tweaked the discover_git_directory function's API based on Peff's feedback
> > * reordered the last three patches so that they flowed a bit better
> > * renamed 'git_config_with_options'
> > * rebased ontop of v4 of Dscho's alias series
> >   
> > https://public-inbox.org/git/cover.1497440104.git.johannes.schinde...@gmx.de/
> 
> Applying this series was messier than necessary, I'd have to say, as
> this series would not apply cleanly on top of the result of applying
> Dscho's v4 on top of the same base as Dscho's v3 was applied (which
> is v2.13.0).  It applied cleanly only when Dscho's v4 and then this
> series were applied on top of 02a2850a ("Sync with maint",
> 2017-06-13), which is a much newer commit than v2.13.0.
> 
> Which in turn means these two fixes cannot be merged to 'maint' as
> you two collaboratively prepared.
> 
> I've applied Dscho's v4 on top of v2.13.0 (just like his v3 was
> queued in my tree on the same base), and then tweaked this series to
> apply on top of that, so that they can go to 'maint' if we chose to.
> 
> But it is possible that during this unnecessary patch shuffling, I
> may have made some mistake, so please eyeball the resulting 12
> patches carefully when they are pushed out.

Ugh, I'm terribly sorry.  Completely my bad as I didn't consider what
you would need to do on your end.  When I built my patches on top of his
I naively just applied his v4 to what ever the current origin/master was
at that point in time.  I'll be sure to be more careful with this
next time.

-- 
Brandon Williams


Re: [PATCH v3 0/6] config.h

2017-06-15 Thread Junio C Hamano
Brandon Williams  writes:

> Changes in v3:
>
> * tweaked the discover_git_directory function's API based on Peff's feedback
> * reordered the last three patches so that they flowed a bit better
> * renamed 'git_config_with_options'
> * rebased ontop of v4 of Dscho's alias series
>   
> https://public-inbox.org/git/cover.1497440104.git.johannes.schinde...@gmx.de/

Applying this series was messier than necessary, I'd have to say, as
this series would not apply cleanly on top of the result of applying
Dscho's v4 on top of the same base as Dscho's v3 was applied (which
is v2.13.0).  It applied cleanly only when Dscho's v4 and then this
series were applied on top of 02a2850a ("Sync with maint",
2017-06-13), which is a much newer commit than v2.13.0.

Which in turn means these two fixes cannot be merged to 'maint' as
you two collaboratively prepared.

I've applied Dscho's v4 on top of v2.13.0 (just like his v3 was
queued in my tree on the same base), and then tweaked this series to
apply on top of that, so that they can go to 'maint' if we chose to.

But it is possible that during this unnecessary patch shuffling, I
may have made some mistake, so please eyeball the resulting 12
patches carefully when they are pushed out.

Thanks.
>


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-08 Thread Liam Beguin
Hi,

On Mon, 2017-05-08 at 09:27 +0900, Junio C Hamano wrote:
> Liam Beguin  writes:
> 
> > Sorry for the delay, I don't mind switching to C but it would probably
> > be easier to see if the scripted version gets approved first.
> > If it does, I could then get started on the C implementation.
> > If you prefer I could also forget about the scripted version, make a C
> > implementation work and see if that gets approved.
> 
> I am not sure what "approved" would mean in the context of this
> project, though ;-) Your patch to the scripted version would
> certainly not be in the upcoming release.  If you define the
> "approval" as "it is queued to my tree somewhere", the patch would
> start its life like everybody else by getting merged to the 'pu'
> branch, where there already is a topic that removes the code you
> patch your enhancement into.
> 

By "approved", I guess I meant the list reaches an agreement. 

> The list _can_ agree that it is a good idea to have an option to
> populate the todo list with shortened insn words from the beginning
> (instead of merely accepting a short-hand while executing), which is
> what your patch wants to do, without actually having the updated
> scripted "rebase -i" merged in any of the integration branches in my
> tree.  If you meant by "approval" to have such a list concensus, I
> think you may already have one.  I personally do not think it is a
> great idea but I do not think it is a horrible one, either.  As long
> as it is an opt-in feature that many people find useful (which may
> be the case already, judging from the list traffic), I do not mind
> ;-)
> 

Ok, based on this, I'll send a new series based on the 'pu' branch.

Thanks again, 
Liam


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-07 Thread Junio C Hamano
Liam Beguin  writes:

> Sorry for the delay, I don't mind switching to C but it would probably
> be easier to see if the scripted version gets approved first.
> If it does, I could then get started on the C implementation.
> If you prefer I could also forget about the scripted version, make a C
> implementation work and see if that gets approved.

I am not sure what "approved" would mean in the context of this
project, though ;-) Your patch to the scripted version would
certainly not be in the upcoming release.  If you define the
"approval" as "it is queued to my tree somewhere", the patch would
start its life like everybody else by getting merged to the 'pu'
branch, where there already is a topic that removes the code you
patch your enhancement into.

The list _can_ agree that it is a good idea to have an option to
populate the todo list with shortened insn words from the beginning
(instead of merely accepting a short-hand while executing), which is
what your patch wants to do, without actually having the updated
scripted "rebase -i" merged in any of the integration branches in my
tree.  If you meant by "approval" to have such a list concensus, I
think you may already have one.  I personally do not think it is a
great idea but I do not think it is a horrible one, either.  As long
as it is an opt-in feature that many people find useful (which may
be the case already, judging from the list traffic), I do not mind
;-)





Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-07 Thread Liam Beguin
Hi Junio,

On Wed, 2017-05-03 at 22:04 -0700, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
> > > If 'git-rebase--interactive.sh' is bound to be replaced, I could
> > > just shrink this to the Documentation cleanup (patches 4 and 5)
> > > and rework the rest on top of your new implementation.
> > 
> > I kind of hoped that Junio would chime in with his verdict. That would be
> > the ultimate deciding factor, I think.
> 
> What I can predict is that within two or three release cycles
> (unless you completely lose interest) the todo-list generation will
> be all in C and that I anticipate that the C version may even be
> capable of generating different kind of todo command (e.g. to
> support tools like your Garden Shears more natively), so the
> mid-term direction definitely is that any enhancement would in the
> end needs to happen on top of or in coordination with the C rewrite
> we've been discussing recently.
> 
> I didn't know what the comfort levels of Liam working with scripted
> vs C code, and the "vertict" depends on that, I would think.  We may
> want to discuss the enhancement in the original scripted form Liam
> did with new tests while the C rewrite is still cooking, and either
> Liam, you or somebody else can make it work with your C rewrite when
> both are ready.  If Liam feels comfortable working with you and the
> code after the C rewrite that is still in-flight, it is also fine if
> the next iteration from Liam were on top of your series.
> 
> 

Sorry for the delay, I don't mind switching to C but it would probably
be easier to see if the scripted version gets approved first.
If it does, I could then get started on the C implementation.
If you prefer I could also forget about the scripted version, make a C
implementation work and see if that gets approved.

Thanks,
Liam


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-03 Thread Junio C Hamano
Johannes Schindelin  writes:

>> If 'git-rebase--interactive.sh' is bound to be replaced, I could
>> just shrink this to the Documentation cleanup (patches 4 and 5)
>> and rework the rest on top of your new implementation.
>
> I kind of hoped that Junio would chime in with his verdict. That would be
> the ultimate deciding factor, I think.

What I can predict is that within two or three release cycles
(unless you completely lose interest) the todo-list generation will
be all in C and that I anticipate that the C version may even be
capable of generating different kind of todo command (e.g. to
support tools like your Garden Shears more natively), so the
mid-term direction definitely is that any enhancement would in the
end needs to happen on top of or in coordination with the C rewrite
we've been discussing recently.

I didn't know what the comfort levels of Liam working with scripted
vs C code, and the "vertict" depends on that, I would think.  We may
want to discuss the enhancement in the original scripted form Liam
did with new tests while the C rewrite is still cooking, and either
Liam, you or somebody else can make it work with your C rewrite when
both are ready.  If Liam feels comfortable working with you and the
code after the C rewrite that is still in-flight, it is also fine if
the next iteration from Liam were on top of your series.




Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-03 Thread Johannes Schindelin
Hi Liam,

On Tue, 2 May 2017, Liam Beguin wrote:

> On Tue, 2017-05-02 at 17:48 +0200, Johannes Schindelin wrote:
> 
> > I offered a couple of comments, my biggest one probably being that
> > this patch series is crossing paths with my patch series that tries to
> > move more functionality out of the git-rebase--interactive.sh script
> > into the git-rebase--helper that is written in C, closely followed by
> > my suggestion to fold at least part of the functionality into the
> > SHA-1 collapsing/expanding.
> 
> I've seen a few messages about this migration, but I'm not yet sure I
> understand the difference between the shell and the C implementations.
> Is the C version going to replace 'git-rebase--interactive.sh'?

The C version has already started to replace the shell script, yes. In
adding and using git-rebase--helper, there are already parts of the
interactive rebase functionality that are run using C code only. The idea
is to move more and more functionality over (separating out the
--preserve-merges handling into a different shell script, as I have no
plans to convert that code to C, and as far as I can see nobody else wants
to step up to that task, either). Eventually, we may be able to finish
that gigantic task of having git-rebase be all builtin C code.

> > If your patch series "wins", I can easily forward-port your changes to
> > the rebase-i-extra branch, but it may actually make sense to build on
> > top of the rebase-i-extra branch to begin with. If you agree: I pushed
> > the proposed change to the `rebase-i-extra+abbrev` branch at
> > https://github.com/dscho/git.
> 
> If 'git-rebase--interactive.sh' is bound to be replaced, I could
> just shrink this to the Documentation cleanup (patches 4 and 5)
> and rework the rest on top of your new implementation.

I kind of hoped that Junio would chime in with his verdict. That would be
the ultimate deciding factor, I think.

Ciao,
Johannes


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-02 Thread Liam Beguin
Hi Johannes, 

On Tue, 2017-05-02 at 17:48 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Tue, 2 May 2017, Liam Beguin wrote:
> 
> > Add the 'rebase.abbreviateCommands' configuration option to allow `git
> > rebase -i` to default to the single-letter command-names in the todo
> > list.
> > 
> > Using single-letter command-names can present two benefits.  First, it
> > makes it easier to change the action since you only need to replace a
> > single character (i.e.: in vim "r" instead of
> > "ciw").  Second, using this with a large enough value of
> > 'core.abbrev' enables the lines of the todo list to remain aligned
> > making the files easier to read.
> > 
> > Changes from v1 to v2:
> >  - Improve Documentation and commit message
> > 
> > Changes from v2 to v3:
> >  - Transform a single patch into a series
> >  - change option name from 'rebase.abbrevCmd' to 'rebase.abbreviateCommands'
> >  - abbreviate all commands (not just pick)
> >  - teach `git rebase -i --autosquash` to recognise single-letter 
> > command-names
> >  - move rebase configuration documentation to 
> > Documentation/rebase-config.txt
> >  - update Documentation to use the preferred naming for the todo list
> >  - update Documentation and commit messages according to feedback
> 
> Thank you for this pleasant read. It is an excellent contribution, and the
> way you communicate what you changed and why is very welcome.
> 

Thanks! and thank you for the support and help.

> I offered a couple of comments, my biggest one probably being that this
> patch series is crossing paths with my patch series that tries to move
> more functionality out of the git-rebase--interactive.sh script into the
> git-rebase--helper that is written in C, closely followed by my suggestion
> to fold at least part of the functionality into the SHA-1
> collapsing/expanding.
> 

I've seen a few messages about this migration, but I'm not yet sure I understand
the difference between the shell and the C implementations. Is the C version 
going
to replace 'git-rebase--interactive.sh'?

> If your patch series "wins", I can easily forward-port your changes to the
> rebase-i-extra branch, but it may actually make sense to build on top of
> the rebase-i-extra branch to begin with. If you agree: I pushed the
> proposed change to the `rebase-i-extra+abbrev` branch at
> https://github.com/dscho/git.
> 

If 'git-rebase--interactive.sh' is bound to be replaced, I could
just shrink this to the Documentation cleanup (patches 4 and 5)
and rework the rest on top of your new implementation.

> I look forward to see this story unfold!
> 
> Ciao,
> Johannes

Thanks, 
Liam


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-02 Thread Johannes Schindelin
Hi Liam,

On Tue, 2 May 2017, Liam Beguin wrote:

> Add the 'rebase.abbreviateCommands' configuration option to allow `git
> rebase -i` to default to the single-letter command-names in the todo
> list.
> 
> Using single-letter command-names can present two benefits.  First, it
> makes it easier to change the action since you only need to replace a
> single character (i.e.: in vim "r" instead of
> "ciw").  Second, using this with a large enough value of
> 'core.abbrev' enables the lines of the todo list to remain aligned
> making the files easier to read.
> 
> Changes from v1 to v2:
>  - Improve Documentation and commit message
> 
> Changes from v2 to v3:
>  - Transform a single patch into a series
>  - change option name from 'rebase.abbrevCmd' to 'rebase.abbreviateCommands'
>  - abbreviate all commands (not just pick)
>  - teach `git rebase -i --autosquash` to recognise single-letter command-names
>  - move rebase configuration documentation to Documentation/rebase-config.txt
>  - update Documentation to use the preferred naming for the todo list
>  - update Documentation and commit messages according to feedback

Thank you for this pleasant read. It is an excellent contribution, and the
way you communicate what you changed and why is very welcome.

I offered a couple of comments, my biggest one probably being that this
patch series is crossing paths with my patch series that tries to move
more functionality out of the git-rebase--interactive.sh script into the
git-rebase--helper that is written in C, closely followed by my suggestion
to fold at least part of the functionality into the SHA-1
collapsing/expanding.

If your patch series "wins", I can easily forward-port your changes to the
rebase-i-extra branch, but it may actually make sense to build on top of
the rebase-i-extra branch to begin with. If you agree: I pushed the
proposed change to the `rebase-i-extra+abbrev` branch at
https://github.com/dscho/git.

I look forward to see this story unfold!

Ciao,
Johannes


Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-02 Thread Johannes Schindelin
Hi,


On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:

> I locally rebased this into just 3 patches, i.e. in this sequence:
> 
> - Documentation: move rebase.* config variables to a separate 
> rebase-config.txt
> - Documentation: use preferred name for the 'todo list' script
> - *all the rest of this squashed*

I think that makes a lot of sense. (I would drop the part about f!/s!, as
I pointed out, though)

Ciao,
Johannes

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

2017-05-02 Thread Ævar Arnfjörð Bjarmason
On Tue, May 2, 2017 at 6:00 AM, Liam Beguin  wrote:
> Add the 'rebase.abbreviateCommands' configuration option to allow
> `git rebase -i` to default to the single-letter command-names in
> the todo list.
>
> Using single-letter command-names can present two benefits.
> First, it makes it easier to change the action since you only need to
> replace a single character (i.e.: in vim "r" instead of
> "ciw").
> Second, using this with a large enough value of 'core.abbrev' enables the
> lines of the todo list to remain aligned making the files easier to
> read.
>
> Changes from v1 to v2:
>  - Improve Documentation and commit message
>
> Changes from v2 to v3:
>  - Transform a single patch into a series
>  - change option name from 'rebase.abbrevCmd' to 'rebase.abbreviateCommands'
>  - abbreviate all commands (not just pick)
>  - teach `git rebase -i --autosquash` to recognise single-letter command-names
>  - move rebase configuration documentation to Documentation/rebase-config.txt
>  - update Documentation to use the preferred naming for the todo list
>  - update Documentation and commit messages according to feedback
>
> Liam Beguin (6):
>   rebase -i: add abbreviated command-names handling
>   rebase -i: add abbreviate_commands function
>   rebase -i: add short command-name in --autosquash
>   Documentation: move rebase.* config variables to a separate
> rebase-config.txt
>   Documentation: use prefered name for the 'todo list' script
>   Documentation: document the rebase.abbreviateCommands option

I locally rebased this into just 3 patches, i.e. in this sequence:

- Documentation: move rebase.* config variables to a separate rebase-config.txt
- Documentation: use preferred name for the 'todo list' script
- *all the rest of this squashed*

I think that's much less confusing than having 3x "rebase -i" patches.
If you look at any one of those you have very little context for
what's going on, and there seems to be no point in splitting them
since the end result is tiny (3 files changed, 45 insertions(+), 4
deletions(-)).

I think with that this looks good, but it also needs tests, if you
apply your series and then comment out the new calls to
abbreviate_commands all tests still pass, if you look at git-config(1)
and search for the other rebase.* commands & grep the test suite for
those you can see how they're tested for.

I don't think this needs a lot of testing since it's a rather trivial
feature, but just one test to make sure that the todo list ends up as
"p ..." "e  ..." instead of "pick ..." "exec ..." etc. would be good.

>  Documentation/config.txt| 31 +---
>  Documentation/git-rebase.txt| 21 +++-
>  Documentation/rebase-config.txt | 53 
> +
>  git-rebase--interactive.sh  | 24 
>  4 files changed, 78 insertions(+), 52 deletions(-)
>  create mode 100644 Documentation/rebase-config.txt
>
> --
> 2.9.3
>


Re: [PATCH v3 0/6] recursively grep across submodules

2016-11-15 Thread Stefan Beller
coverity seems to dislike this part:

*** CID 1394367:  Null pointer dereferences  (NULL_RETURNS)
/builtin/grep.c: 625 in grep_submodule()
619   is_submodule_populated(path))) {
620 /*
621  * If searching history, check for the presense of the
622  * submodule's gitdir before skipping the submodule.
623  */
624 if (sha1) {
>>> CID 1394367:  Null pointer dereferences  (NULL_RETURNS)
>>> Dereferencing a null pointer "submodule_from_path(null_sha1, path)".
625 path = git_path("modules/%s",
626
submodule_from_path(null_sha1, path)->name);
627
628 if (!(is_directory(path) &&
is_git_directory(path)))
629 return 0;
630 } else {


Re: [PATCH v3 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c

2016-10-05 Thread Johannes Schindelin
Hi Junio,

On Tue, 4 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > This is the 5th last patch series of my work to accelerate interactive
> > rebases in particular on Windows.
> 
> Offtopic, but I am always confused by what you might mean by this
> "nth last patch series".  Is this series 5th from the last and we
> have four more to go?

Yes, we have four more to go:

- the patch series (called "prepare-sequencer" in my fork) preparing the
  sequencer code for the next patch series, such as revamping the parser
  for the edit script (or "todo script" as I used to say all the time, or
  "insn sheet" as sequencer calls it),

- the patch series ("sequencer-i") teaching the sequencer to parse and
  execute the commands of rebase -i's git-rebase-todo file,

- the patch series ("rebase--helper") introducing the builtin, and using
  it from rebase -i, and finally

- the patch series ("rebase-i-extra") that moves more performance critical
  bits and pieces from git-rebase--interactive.sh into the rebase--helper.

I had originally planned to stop at rebase--helper and invite other
developers to join the fun of making rebase -i a true builtin, but the
performance improvement was surprisingly disappointing before the
rebase--helper learned to skip unnecessary picks, to verify that the
script is valid, to expand/collapse the SHA-1s, and to rearrange
fixup!/squash!  lines.

> In any case, after a quick re-read and comparison with the last
> round, I think this is in a good shape.  I'd say that we would wait
> for a few days for others to comment and then merge it to 'next' if
> we missed nothing glaringly wrong.

Perfect!
Dscho


Re: [PATCH v3 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c

2016-10-04 Thread Junio C Hamano
Johannes Schindelin  writes:

> This is the 5th last patch series of my work to accelerate interactive
> rebases in particular on Windows.

Offtopic, but I am always confused by what you might mean by this
"nth last patch series".  Is this series 5th from the last and we
have four more to go?

In any case, after a quick re-read and comparison with the last
round, I think this is in a good shape.  I'd say that we would wait
for a few days for others to comment and then merge it to 'next' if
we missed nothing glaringly wrong.

Thanks.


Re: [PATCH v3 0/6] Split .git/config in multiple worktree setup

2016-01-27 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> For any worktree, the new file .git/common/config is read first, then
> either .git/config or .git/worktrees/xxx/config is read after. There's
> no special per-worktree var list any more. Which is great. You want to
> add per-worktree config vars, use "git config --local". You want to
> add per-repo config vars, use (new) "git config --repo". You put a
> variable in a wrong file, you're punished for it (and it's the same
> today, say if you put core.worktree to /etc/gitconfig).

I think this is saner than the one before.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] fix repo name when cloning a server's root

2015-08-06 Thread Torsten Bögershausen
On 2015-08-05 23.19, Jeff King wrote:
 On Wed, Aug 05, 2015 at 10:34:34AM -0700, Junio C Hamano wrote:
 
 As you can see, there is a lot of complexity in there and I'm not
 convinced this is better than just exposing
 'parse_connect_url()', which already handles everything for us.
I try expose and use parse_connect_url():
It handles the scp-like syntax host:/path,
literall IPV6 addresses, port numbers,
':' without a port number and all other Git specific parsing,
which is inside and outside the RFC 3986.
(I should know, because I managed to break the parser twice,
and fix it)

I added a diagnostics to connect.c, and if you run the a simply test,
we can see that the colon slash logic is often unsufficient:

tb@mypc:~/projects/git/tb.150731_connect ./git fetch-pack --diag-url 
ssh://host/
Diag: url=ssh://host/
Diag: protocol=ssh
Diag: userandhost=host
Diag: port=NONE
Diag: path=/
Diag: guesseddir=host/
tb@macce:~/projects/git/tb.150731_connect ./git fetch-pack --diag-url 
ssh://host:/
Diag: url=ssh://host:/
Diag: protocol=ssh
Diag: userandhost=host
Diag: port=NONE
Diag: path=/
Diag: guesseddir=/


On top of that, you can easily write test cases in t5601, as many as you want.
The (minor) drawback is that it doesn't handle http:// or https://,
but that is easy to add in the parser, and doesn't break existing code.

The major which remains is to search for '@' in userandhost,
and strip that off.
(Or when there is a '@', search for a ':' before the '@', and strip that off)
After that, all non-printable characters should be %-escaped.
If we replace ':' as non-printable as well, we can make Windows users 1% more 
happy.



 If the function handles everything for us, that's fine, but the
 primary reason I am hesitant is because parse_connect_url() was
 designed specifically not to have to worry about some protocols
 (e.g. I think feeding it a http://; would fail, and more
 importantly, its current callers want such a call to fail).  Also it
 is meant to handle some non-protocols (e.g. scp style host:path that
 does not follow scheme://...).
 
 True, but the transport code _is_ handling that at some point. It makes
 me wonder if it would be possible to push the call to transport_get
 further up inside cmd_clone(), and then provide some way to query the
 remote path and hostname from the transport code. Then guess_dir_name
 could just go away entirely, in favor of something like:
 
   dir_name = transport_get_path(transport);
   if (!*dir_name)
   dir_name = transport_get_host(transport);
 
 That may be overly simplistic or unworkable, though. I haven't dug into
 the code.
 
 Also does it handle the  case above?  I do not think
 parse_connect_url() even calls get_host_and_port() to be able to
 tell what  means in these examples.
 
 Speaking of which, has anyone tested whether the old or new code handles
 external remote helpers? Certainly:
 
   foo::https://host/repo.git
 
 should still use repo.git. But technically the string handed to
 git-remote-foo does not have to look anything like a URL. In those cases
 neither guess_dir_name nor the transport code have any idea what anything
 to the right of the :: means; we probably have to resort to blind
 guessing based on characters like colon and slash.
 
It is easy to strip the foo:: part of the url, assume that
the remote helper uses a RFC 3986 similar url syntax, so that we
can feed the reminding https://host/repo.git into the parser (see above).

If the remote helper doesn't do this, we can't guess anything, can we ?
So error out and tell the user seems the right thing to do.

In the hope that this is useful, pushed my prototype branch to
https://github.com/tboegi/git/tree/150731_connect_diag_guess_name









--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] fix repo name when cloning a server's root

2015-08-06 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 It is easy to strip the foo:: part of the url, assume that
 the remote helper uses a RFC 3986 similar url syntax, so that we
 can feed the reminding https://host/repo.git into the parser (see above).

The thing that worries me is that foo:: syntax and external helper
interface was invented by Daniel Barkalow primarily because he
wanted to allow things that do *not* fit in that URL-like scheme,
for example see:

  http://thread.gmane.org/gmane.comp.version-control.git/125374/focus=125405

 If the remote helper doesn't do this, we can't guess anything, can we ?
 So error out and tell the user seems the right thing to do.

Yes.  A blind guess that fails spectacularly is far worse than an
outright rejection that is cautious.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] fix repo name when cloning a server's root

2015-08-05 Thread Junio C Hamano
Patrick Steinhardt p...@pks.im writes:

  - The naive way of just adding '@' as path separator would break
cloning repositories like '/foo/b...@baz.git' (which would
currently become 'bar@baz' but would become 'baz' only).

  - Skipping the scheme initially is required because without it we
wouldn't be able to scan until the next dir separator in the
host part when stripping authentication information.

  - First checking for '/' in the current stripped URI when we
want to remove the port is required because we do not want to
strip port numbers when cloning from something like
'/foo/bar:.git' (which would currently become '' but
would then be stripped of the ':' part and instead become
'bar'). Still, this breaks on cloning a bare repository in the
current dir (e.g. cloning 'bar:.git', which should become
'' because it is not a port number but would become
'bar').

This is a very good write-up.

Please make sure all of the above appears in the commit log message
somewhere.

 As you can see, there is a lot of complexity in there and I'm not
 convinced this is better than just exposing
 'parse_connect_url()', which already handles everything for us.

If the function handles everything for us, that's fine, but the
primary reason I am hesitant is because parse_connect_url() was
designed specifically not to have to worry about some protocols
(e.g. I think feeding it a http://; would fail, and more
importantly, its current callers want such a call to fail).  Also it
is meant to handle some non-protocols (e.g. scp style host:path that
does not follow scheme://...).

Also does it handle the  case above?  I do not think
parse_connect_url() even calls get_host_and_port() to be able to
tell what  means in these examples.

 Maybe I'm just being blind for the obvious solution here, though.

 Patrick Steinhardt (6):
   tests: fix broken  chains in t1509-root-worktree
   tests: fix cleanup after tests in t1509-root-worktree
   clone: do not include authentication data in guessed dir
   clone: do not use port number as dir name
   clone: abort if no dir name could be guessed
   clone: add tests for cloning with empty path

  builtin/clone.c  | 67 
 
  t/t1509-root-worktree.sh | 51 +---
  2 files changed, 103 insertions(+), 15 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] fix repo name when cloning a server's root

2015-08-05 Thread Jeff King
On Wed, Aug 05, 2015 at 10:34:34AM -0700, Junio C Hamano wrote:

  As you can see, there is a lot of complexity in there and I'm not
  convinced this is better than just exposing
  'parse_connect_url()', which already handles everything for us.
 
 If the function handles everything for us, that's fine, but the
 primary reason I am hesitant is because parse_connect_url() was
 designed specifically not to have to worry about some protocols
 (e.g. I think feeding it a http://; would fail, and more
 importantly, its current callers want such a call to fail).  Also it
 is meant to handle some non-protocols (e.g. scp style host:path that
 does not follow scheme://...).

True, but the transport code _is_ handling that at some point. It makes
me wonder if it would be possible to push the call to transport_get
further up inside cmd_clone(), and then provide some way to query the
remote path and hostname from the transport code. Then guess_dir_name
could just go away entirely, in favor of something like:

  dir_name = transport_get_path(transport);
  if (!*dir_name)
dir_name = transport_get_host(transport);

That may be overly simplistic or unworkable, though. I haven't dug into
the code.

 Also does it handle the  case above?  I do not think
 parse_connect_url() even calls get_host_and_port() to be able to
 tell what  means in these examples.

Speaking of which, has anyone tested whether the old or new code handles
external remote helpers? Certainly:

  foo::https://host/repo.git

should still use repo.git. But technically the string handed to
git-remote-foo does not have to look anything like a URL. In those cases
neither guess_dir_name nor the transport code have any idea what anything
to the right of the :: means; we probably have to resort to blind
guessing based on characters like colon and slash.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] pseudorefs

2015-07-28 Thread Junio C Hamano
On top of what work is this series expected to be applied?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] pseudorefs

2015-07-28 Thread David Turner
On Tue, 2015-07-28 at 12:01 -0700, Junio C Hamano wrote:
 On top of what work is this series expected to be applied?

I think I started from 'next' as of a few days ago:

commit df7aaa5e3454bbcbb1f142dd6b95b214d0b8efad
Author: Zoë Blade z...@bytenoise.co.uk
Date:   Tue Jul 21 14:22:46 2015 +0100

userdiff: add support for Fountain documents

Let me know if you need me to rebase on something else.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] Rewrite `git_config()` using config-set API

2014-07-28 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 [PATCH V3]:All the suggestions in [3] applied. Built on top of [1].

Except for the minor style fix in PATCH 5, the series looks OK to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git_config callers rewritten with the new config cache API

2014-07-21 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 [PATCH v3]: Most of Eric's suggestions has been implemented. See [2] for 
 discussion.
   Also, new helpers introduced in v7 of the config-set API series have 
 been used.
   See [1] for the documentation of the new functions.

 This series builds on the top of 5def4132 in pu or topic[1] in the mailing 
 list
 with name git config cache  special querying API utilizing the cache.

It's now called ta/config-set (see last What's cooking in git.git).

 All patches pass every test, but there is a catch, there is slight behaviour
 change in most of them where originally the callback returns
 config_error_nonbool() when it sees a NULL value for a key causing a die
 specified in git_parse_source in config.c.

 The die also prints the file name and the line number as,

   die(bad config file line %d in %s, cf-linenr, cf-name);

 We lose the fine grained error checking when switching to this method.

I think a first step would be something like this:

--- a/config.c
+++ b/config.c
@@ -656,6 +656,15 @@ int git_config_string(const char **dest, const char *var, 
const char *value)
return 0;
 }
 
+// TODO: either make it static or export it properly
+int git_config_string_or_die(const char **dest, const char *var, const char 
*value)
+{
+   if (git_config_string(dest, var, value)  0)
+   die(bad config file (TODO: file/line info));
+   else
+   return 0;
+}
+
 int git_config_pathname(const char **dest, const char *var, const char *value)
 {
if (!value)
@@ -1336,7 +1345,7 @@ int git_configset_get_string(struct config_set *cs, const 
char *key, const char
 {
const char *value;
if (!git_configset_get_value(cs, key, value))
-   return git_config_string(dest, key, value);
+   return git_config_string_or_die(dest, key, value);
else
return 1;
 }

In the original API, git_config_string was called at parsing time, hence
the file/line information was available through cf. Here, we're
querying the cache which doesn't have this information yet.

I initially thought that managing properly file/line information would
be just an addition, but this example shows that it is actually needed
to be feature-complete wrt the old API. And I think we should be
feature-complete (i.e. make the code cleaner without harming the user).

So, I think it now makes sense to resurect your file line info patch:

  http://article.gmane.org/gmane.comp.version-control.git/253123

Now that the series is properly reviewed, avoid modifying existing
patches as much as possible, and add these file/line info on top of the
existing.

I think you need to:

1) Modify the hashmap data structure and the code that fills it in to
   store the file/line info (already done in your previous WIP patch).

2) Add a by-address parameter to git_configset_get_value that allows the
   user to get the file and line information. In your previous patch,
   that would mean returning a pointer to the corresponding struct
   key_source.

3) Pass this information to git_config_string_or_die, and die with the
   right message (with a helper like die_config(struct key_source *ks)
   that takes care of the formatting)

4) apply the same to git_config_get_other than string.

I'd actually add a step 0) before that: add a test that checks your
behavior change. The test should pass without your patches, and fail
with your current patch. Then, it should pass again once you completed
the work.

On a side note, re-reading your previous patch, I found this which
sounds suspicious:

+   struct config_hash_entry *e;
+   struct string_list_item *si;
+   struct key_source *ks = xmalloc(sizeof(*e));

Didn't you mean xmalloc(sizeof(*ks))?
^^

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git_config callers rewritten with the new config cache API

2014-07-21 Thread Tanay Abhra


On 7/21/2014 6:21 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 [PATCH v3]: Most of Eric's suggestions has been implemented. See [2] for 
 discussion.
  Also, new helpers introduced in v7 of the config-set API series have 
 been used.
  See [1] for the documentation of the new functions.

 This series builds on the top of 5def4132 in pu or topic[1] in the mailing 
 list
 with name git config cache  special querying API utilizing the cache.
 
 It's now called ta/config-set (see last What's cooking in git.git).


Noted. More below.

 All patches pass every test, but there is a catch, there is slight behaviour
 change in most of them where originally the callback returns
 config_error_nonbool() when it sees a NULL value for a key causing a die
 specified in git_parse_source in config.c.

 The die also prints the file name and the line number as,

  die(bad config file line %d in %s, cf-linenr, cf-name);

 We lose the fine grained error checking when switching to this method.
 
 I think a first step would be something like this:
 
 --- a/config.c
 +++ b/config.c
 @@ -656,6 +656,15 @@ int git_config_string(const char **dest, const char 
 *var, const char *value)
 return 0;
  }
  
 +// TODO: either make it static or export it properly
 +int git_config_string_or_die(const char **dest, const char *var, const char 
 *value)
 +{
 +   if (git_config_string(dest, var, value)  0)
 +   die(bad config file (TODO: file/line info));
 +   else
 +   return 0;
 +}
 +
  int git_config_pathname(const char **dest, const char *var, const char 
 *value)
  {
 if (!value)
 @@ -1336,7 +1345,7 @@ int git_configset_get_string(struct config_set *cs, 
 const char *key, const char
  {
 const char *value;
 if (!git_configset_get_value(cs, key, value))
 -   return git_config_string(dest, key, value);
 +   return git_config_string_or_die(dest, key, value);
 else
 return 1;
  }
 
 In the original API, git_config_string was called at parsing time, hence
 the file/line information was available through cf. Here, we're
 querying the cache which doesn't have this information yet.
 
 I initially thought that managing properly file/line information would
 be just an addition, but this example shows that it is actually needed
 to be feature-complete wrt the old API. And I think we should be
 feature-complete (i.e. make the code cleaner without harming the user).
 
 So, I think it now makes sense to resurect your file line info patch:
 
   http://article.gmane.org/gmane.comp.version-control.git/253123
 
 Now that the series is properly reviewed, avoid modifying existing
 patches as much as possible, and add these file/line info on top of the
 existing.
 
 I think you need to:
 
 1) Modify the hashmap data structure and the code that fills it in to
store the file/line info (already done in your previous WIP patch).
 
 2) Add a by-address parameter to git_configset_get_value that allows the
user to get the file and line information. In your previous patch,
that would mean returning a pointer to the corresponding struct
key_source.

Will this extra complexity be good for git_configset_get_value?
Instead can we provide a function like die_config(char *key)
which prints
die(bad config file line %d in %s, linenr, filename);?
A variation would be die_config_multi(char *key, char *value)
for multi valued keys.

 3) Pass this information to git_config_string_or_die, and die with the
right message (with a helper like die_config(struct key_source *ks)
that takes care of the formatting)

No need for passing if we use the above method. We will just call die_config()
inside it for NULL values

 4) apply the same to git_config_get_other than string.

 I'd actually add a step 0) before that: add a test that checks your
 behavior change. The test should pass without your patches, and fail
 with your current patch. Then, it should pass again once you completed
 the work.


Noted, I will add it.

 On a side note, re-reading your previous patch, I found this which
 sounds suspicious:
 
 + struct config_hash_entry *e;
 + struct string_list_item *si;
 + struct key_source *ks = xmalloc(sizeof(*e));
 
 Didn't you mean xmalloc(sizeof(*ks))?


Yikes, Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git_config callers rewritten with the new config cache API

2014-07-21 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 On 7/21/2014 6:21 PM, Matthieu Moy wrote:
 2) Add a by-address parameter to git_configset_get_value that allows the
user to get the file and line information. In your previous patch,
that would mean returning a pointer to the corresponding struct
key_source.

 Will this extra complexity be good for git_configset_get_value?
 Instead can we provide a function like die_config(char *key)
 which prints
   die(bad config file line %d in %s, linenr, filename);?

Where would you call this function, and where would you take linenr and
filename?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git_config callers rewritten with the new config cache API

2014-07-21 Thread Tanay Abhra


On 7/21/2014 7:15 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 On 7/21/2014 6:21 PM, Matthieu Moy wrote:
 2) Add a by-address parameter to git_configset_get_value that allows the
user to get the file and line information. In your previous patch,
that would mean returning a pointer to the corresponding struct
key_source.

 Will this extra complexity be good for git_configset_get_value?
 Instead can we provide a function like die_config(char *key)
 which prints
  die(bad config file line %d in %s, linenr, filename);?
 
 Where would you call this function, and where would you take linenr and
 filename?


Usage can be like this,

if(!git_config_get_value(k, v)) {
if (!v) {
config_error_nonbool(k);
die_config(k);
/* die_config calls git_config_get_value_multi for 'k',
 * gets the string list with the util pointer containing
 * the linenr and the file name, dies printing the message.
 */
} else
/* do work */
}

Above example works just like the current code. Currently the callbacks
does not have the access to the linenr and file name anyway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git_config callers rewritten with the new config cache API

2014-07-21 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 On 7/21/2014 7:15 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 On 7/21/2014 6:21 PM, Matthieu Moy wrote:
 2) Add a by-address parameter to git_configset_get_value that allows the
user to get the file and line information. In your previous patch,
that would mean returning a pointer to the corresponding struct
key_source.

 Will this extra complexity be good for git_configset_get_value?
 Instead can we provide a function like die_config(char *key)
 which prints
 die(bad config file line %d in %s, linenr, filename);?
 
 Where would you call this function, and where would you take linenr and
 filename?


 Usage can be like this,

 if(!git_config_get_value(k, v)) {
   if (!v) {
   config_error_nonbool(k);
   die_config(k);
   /* die_config calls git_config_get_value_multi for 'k',
* gets the string list with the util pointer containing
* the linenr and the file name, dies printing the message.
*/
   } else
   /* do work */
 }

OK, so you query the cache twice (which is OK, it's cheap and happens
just once before dying). That would work too.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6]

2013-07-23 Thread Jakub Narebski
Junio C Hamano gitster at pobox.com writes:

 
 This is mostly unchanged since the previous round, except that
 
  * The option is spelled --force-with-lease=ref:expect.
Nobody liked cas as it was too technical, many disliked
lockref because lock sounded as if push by others were
excluded by it while in fact this is to fail us.

Perhaps --force-gently ? :-)

-- 
Jakub Narębski

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6]

2013-07-23 Thread Junio C Hamano
Jakub Narebski jna...@gmail.com writes:

 Junio C Hamano gitster at pobox.com writes:

 
 This is mostly unchanged since the previous round, except that
 
  * The option is spelled --force-with-lease=ref:expect.
Nobody liked cas as it was too technical, many disliked
lockref because lock sounded as if push by others were
excluded by it while in fact this is to fail us.

 Perhaps --force-gently ? :-)

Hmph.  But we usually use gently to mean do not give the end user
an error message--the caller handles the error itself.

While the option lets you break the usual must fast-forward rule,
it is more precise in that the remote ref must be pointing at not
just any ancestor of what you are pushing, but has to be at the
exact commit you specify.

E.g. if you have built one commit on top of the shared branch, and
try to push it with push --cas=pu:HEAD^ HEAD:pu (because you know
one commit before the tip is where you started from), your push will
be rejected if somebody else did an equivalent of reset HEAD~3 on
the receiving end (perhaps because the top commits recorded some
material inappropriate for the project).  Your new commit is still a
decendant of that rewound tip, and usual must fast-forward rule
would accept the push, but with push --cas=pu:HEAD^ HEAD:pu, you
will notice that somebody wanted to rewind the tip and pushing your
work that contains these dropped commit contradicts with that wish.

So I dunno.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6]

2013-07-23 Thread Philip Oakley

From: Junio C Hamano gits...@pobox.com
Sent: Tuesday, July 23, 2013 7:26 PM

Jakub Narebski jna...@gmail.com writes:


Junio C Hamano gitster at pobox.com writes:



This is mostly unchanged since the previous round, except that

 * The option is spelled --force-with-lease=ref:expect.
   Nobody liked cas as it was too technical, many disliked
   lockref because lock sounded as if push by others were
   excluded by it while in fact this is to fail us.


Perhaps --force-gently ? :-)


Or --force-carefully to better indicate the safety / care that is 
being applied?




Hmph.  But we usually use gently to mean do not give the end user
an error message--the caller handles the error itself.

While the option lets you break the usual must fast-forward rule,
it is more precise in that the remote ref must be pointing at not
just any ancestor of what you are pushing, but has to be at the
exact commit you specify.

E.g. if you have built one commit on top of the shared branch, and
try to push it with push --cas=pu:HEAD^ HEAD:pu (because you know
one commit before the tip is where you started from), your push will
be rejected if somebody else did an equivalent of reset HEAD~3 on
the receiving end (perhaps because the top commits recorded some
material inappropriate for the project).  Your new commit is still a
decendant of that rewound tip, and usual must fast-forward rule
would accept the push, but with push --cas=pu:HEAD^ HEAD:pu, you
will notice that somebody wanted to rewind the tip and pushing your
work that contains these dropped commit contradicts with that wish.

So I dunno.
--


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6]

2013-07-23 Thread Eric Sunshine
On Tue, Jul 23, 2013 at 5:26 PM, Philip Oakley philipoak...@iee.org wrote:
 From: Junio C Hamano gits...@pobox.com
 Sent: Tuesday, July 23, 2013 7:26 PM
 Jakub Narebski jna...@gmail.com writes:
 Junio C Hamano gitster at pobox.com writes:
 This is mostly unchanged since the previous round, except that

  * The option is spelled --force-with-lease=ref:expect.
Nobody liked cas as it was too technical, many disliked
lockref because lock sounded as if push by others were
excluded by it while in fact this is to fail us.

 Perhaps --force-gently ? :-)

 Or --force-carefully to better indicate the safety / care that is being
 applied?

[bike-shedding: on]

--force-if-safe

[bike-shedding: off]
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

2013-06-04 Thread Junio C Hamano
Michael S. Tsirkin m...@redhat.com writes:

 This includes bugfixes related to handling of --suppress-cc=self
 flag. Tests are also included.

Hmph, flipped the patches without test-applying first?

2/6 adds two lines to test_suppress_self_quoted helper function, but
that is introduced only at 4/6.


 Changes from v2:
   - add a new test, split patches differently add code comments
to address comments by Junio
   - rename example addresses in tests from redhat.com to example.com
 Changes from v1:
 - tweak coding style in tests to address comments by Junio


 Michael S. Tsirkin (6):
   send-email: fix suppress-cc=self on cccmd
   t/send-email: test suppress-cc=self on cccmd
   send-email: make --suppress-cc=self sanitize input
   t/send-email: add test with quoted sender
   t/send-email: test suppress-cc=self with non-ascii
   test-send-email: test for pre-sanitized self name

  git-send-email.perl   | 23 +++
  t/t9001-send-email.sh | 34 +-
  2 files changed, 48 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

2013-06-04 Thread Michael S. Tsirkin
On Tue, Jun 04, 2013 at 10:40:51AM -0700, Junio C Hamano wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  This includes bugfixes related to handling of --suppress-cc=self
  flag. Tests are also included.
 
 Hmph, flipped the patches without test-applying first?

No, I generated the patches with git format-patch,
then processed with git send-email.

 2/6 adds two lines to test_suppress_self_quoted helper function, but
 that is introduced only at 4/6.

I don't see it
All I see in 2/6 is this:

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e1a7f3e..f81e5f5 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {

unquoted-$3

+   cccmd--$1 $2
+
Cc: $1 $2
Signed-off-by: $1 $2
EOF
 }

 test_expect_success $PREREQ 'self name is suppressed' 
-   test_suppress_self_unquoted 'A U Thor' 'aut...@redhat.com' \
+   test_suppress_self_unquoted 'A U Thor' 'aut...@example.com' \
'self_name_suppressed'
 

where's test_suppress_self_quoted here?


 
  Changes from v2:
  - add a new test, split patches differently add code comments
   to address comments by Junio
  - rename example addresses in tests from redhat.com to example.com
  Changes from v1:
  - tweak coding style in tests to address comments by Junio
 
 
  Michael S. Tsirkin (6):
send-email: fix suppress-cc=self on cccmd
t/send-email: test suppress-cc=self on cccmd
send-email: make --suppress-cc=self sanitize input
t/send-email: add test with quoted sender
t/send-email: test suppress-cc=self with non-ascii
test-send-email: test for pre-sanitized self name
 
   git-send-email.perl   | 23 +++
   t/t9001-send-email.sh | 34 +-
   2 files changed, 48 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

2013-06-04 Thread Junio C Hamano
Michael S. Tsirkin m...@redhat.com writes:

 All I see in 2/6 is this:

   diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
   index e1a7f3e..f81e5f5 100755
   --- a/t/t9001-send-email.sh
   +++ b/t/t9001-send-email.sh
   @@ -204,13 +204,15 @@ test_suppress_self_unquoted () {

After noticing what is after @@ here...


   unquoted-$3

   +   cccmd--$1 $2
   +
   Cc: $1 $2
   Signed-off-by: $1 $2
   EOF
}

test_expect_success $PREREQ 'self name is suppressed' 
   -   test_suppress_self_unquoted 'A U Thor' 'aut...@redhat.com' \
   +   test_suppress_self_unquoted 'A U Thor' 'aut...@example.com' \
   'self_name_suppressed'


 where's test_suppress_self_quoted here?

... isn't addition of cccmd--$1 $2 made to that function?

After applying [PATCH v3 1/6] to 'master', I do not see unquoted-$3
that we see in the context, either.

git grep unquoted- pu t/t9001* does show us a hit, but that is
coming from your previous round you are replacing with this series,
so

Confused...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] completion: test consolidations

2012-11-19 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 These started from a discussion with SZEDER, but then I realized there were
 many improvements possible.

Thanks; I'd loop him in and wait for his acks/reviews.


 Changes since v2:

  * Updated comments and commit messages

 Changes since v1:

  * A lot more cleanups

 Felipe Contreras (6):
   completion: add comment for test_completion()
   completion: standardize final space marker in tests
   completion: simplify tests using test_completion_long()
   completion: consolidate test_completion*() tests
   completion: refactor __gitcomp related tests
   completion: simplify __gitcomp() test helper

  t/t9902-completion.sh | 133 
 +++---
  1 file changed, 51 insertions(+), 82 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/6] completion: test consolidations

2012-11-19 Thread Felipe Contreras
On Mon, Nov 19, 2012 at 9:34 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 These started from a discussion with SZEDER, but then I realized there were
 many improvements possible.

 Thanks; I'd loop him in and wait for his acks/reviews.

I thought my series-cc-cmd would add him. Maybe I'm using a version of
'git send-email' that doesn't have that.

Either way, we already know what he will say, to the previous series
he said he was against consolidation of test scripts. So I don't see
how the situation would change.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html