Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
On Mon, May 7, 2018 at 10:50 AM, SZEDER Gáborwrote: >> 2) Your completion commands for branch-diff will only complete one >> revision range, not two. e.g. >> git branch-diff origin/master..my-topic@{2} origin/master..my-top >> won't complete "my-topic" as I'd expect. > > It does complete two revision ranges, but if you want to look at > reflogs, then you must escape the opening curly brace. I'm not sure > why, but apparently after the unescaped '{' Bash thinks that it's a > new command, and doesn't even call our completion functions anymore. > It's not specific to the completion of 'branch-diff', or even to our > completion script. I don't think we can do anything about it. Ah, indeed. Thanks for the pointer.
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
> 2) Your completion commands for branch-diff will only complete one > revision range, not two. e.g. > git branch-diff origin/master..my-topic@{2} origin/master..my-top > won't complete "my-topic" as I'd expect. It does complete two revision ranges, but if you want to look at reflogs, then you must escape the opening curly brace. I'm not sure why, but apparently after the unescaped '{' Bash thinks that it's a new command, and doesn't even call our completion functions anymore. It's not specific to the completion of 'branch-diff', or even to our completion script. I don't think we can do anything about it.
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Hi Dscho, On Sat, May 5, 2018 at 1:03 PM, Johannes Schindelinwrote: > Hi Elijah, > > On Fri, 4 May 2018, Elijah Newren wrote: > >> - tbdiff aligned output columns better when there were more than 9 >> patches (I'll comment more on patch 09/18) > > I added a new patch to align the patch numbers specifically. I considered > squashing it into 9/18, but decided against it: it will make it easier to > read through the rationale when calling `git annotate` on those lines. Awesome, thanks. >> Also, I don't have bash-completion for either tbdiff or branch-diff. >> :-( But I saw some discussion on the v1 patches about how this gets >> handled... :-) > > Oh? Does 18/18 not work for you? > https://public-inbox.org/git/71698f11835311c103aae565a2a761d10f4676b9.1525448066.git.johannes.schinde...@gmx.de/ It looks like it does work, in part, there were just two issues: 1) I apparently wasn't using all the nice improvements from the completion script in my locally built git, but was instead still using the one associated with my system-installed (and much older) git. (Oops, my bad.) 2) Your completion commands for branch-diff will only complete one revision range, not two. e.g. git branch-diff origin/master..my-topic@{2} origin/master..my-top won't complete "my-topic" as I'd expect. Elijah
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Hi Brian, On Sun, 6 May 2018, brian m. carlson wrote: > On Fri, May 04, 2018 at 05:34:27PM +0200, Johannes Schindelin wrote: > > The incredibly useful `git-tbdiff` tool to compare patch series (say, > > to see what changed between two iterations sent to the Git mailing > > list) is slightly less useful for this developer due to the fact that > > it requires the `hungarian` and `numpy` Python packages which are for > > some reason really hard to build in MSYS2. So hard that I even had to > > give up, because it was simply easier to reimplement the whole shebang > > as a builtin command. > > I just want to say thanks for writing this. I use tbdiff extensively at > work and having this built-in and much faster will really help. > > I did a once-over of v1 and I'll probably take a look at v2 or v3 > (whatever's the latest) later in the week. Thank you so much! Dscho
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
On Fri, May 04, 2018 at 05:34:27PM +0200, Johannes Schindelin wrote: > The incredibly useful `git-tbdiff` tool to compare patch series (say, to see > what changed between two iterations sent to the Git mailing list) is slightly > less useful for this developer due to the fact that it requires the > `hungarian` > and `numpy` Python packages which are for some reason really hard to build in > MSYS2. So hard that I even had to give up, because it was simply easier to > reimplement the whole shebang as a builtin command. I just want to say thanks for writing this. I use tbdiff extensively at work and having this built-in and much faster will really help. I did a once-over of v1 and I'll probably take a look at v2 or v3 (whatever's the latest) later in the week. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Hi Junio, On Sun, 6 May 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > Johannes Schindelin (17): > > Add a function to solve least-cost assignment problems > > Add a new builtin: branch-diff > > Perhaps retitling these to > > hungarian: a function to solve least-cost assignment problems > branch-diff: a new builtin to compare iterations of a topic > > may serve as good precedents to changes other people may later make > to these files. Especially the second one is already consistent > with the several changes that are listed below ;-) I like it! They are retitled locally, in preparation for whenever I send out the next iteration. Ciao, Dscho
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Johannes Schindelinwrites: > Johannes Schindelin (17): > Add a function to solve least-cost assignment problems > Add a new builtin: branch-diff Perhaps retitling these to hungarian: a function to solve least-cost assignment problems branch-diff: a new builtin to compare iterations of a topic may serve as good precedents to changes other people may later make to these files. Especially the second one is already consistent with the several changes that are listed below ;-) > branch-diff: first rudimentary implementation > branch-diff: improve the order of the shown commits > branch-diff: also show the diff between patches >...
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Hi Elijah, On Fri, 4 May 2018, Elijah Newren wrote: > On Fri, May 4, 2018 at 8:34 AM, Johannes Schindelin >wrote: > > The incredibly useful `git-tbdiff` tool to compare patch series (say, to see > > what changed between two iterations sent to the Git mailing list) is > > slightly > > less useful for this developer due to the fact that it requires the > > `hungarian` > > and `numpy` Python packages which are for some reason really hard to build > > in > > MSYS2. So hard that I even had to give up, because it was simply easier to > > reimplement the whole shebang as a builtin command. > > tbdiff is awesome; thanks for bringing it in as a builtin to git. You're welcome. > I've run through a few cases, comparing output of tbdiff and > branch-diff. So far, what I've noted is that they produce largely the > same output except that: > > - tbdiff seems to shorten shas to 7 characters, branch-diff is using > 10, in git.git at least. (Probably a good change) Yes, it is a good change ;-) > - tbdiff aligned output columns better when there were more than 9 > patches (I'll comment more on patch 09/18) I added a new patch to align the patch numbers specifically. I considered squashing it into 9/18, but decided against it: it will make it easier to read through the rationale when calling `git annotate` on those lines. > - As noted elsewhere in the review of round 1, tbdiff uses difflib > while branch-diff uses xdiff. I found some cases where that mattered, > and in all of them, I either felt like the difference was irrelevant > or that difflib was suboptimal, so this is definitely an improvement > for me. Indeed. It is more or less ambiguities that get resolved differently. > - branch-diff produces it's output faster, and it is automatically > paged. This is really cool. :-) It was actually the paging that made the most difference for me. The `git tbdiff` command broke for me as soon as I switched on the pager, as tbdiff got confused by the decoration (AEvar had put up a PR to fix that, but that PR has not even so much as been answered in the meantime, so I thought it'd be a good time to rewrite the entire shebang in C, also because I could not use tbdiff *at all* on Windows due to its hefty dependencies). > Also, I don't have bash-completion for either tbdiff or branch-diff. > :-( But I saw some discussion on the v1 patches about how this gets > handled... :-) Oh? Does 18/18 not work for you? https://public-inbox.org/git/71698f11835311c103aae565a2a761d10f4676b9.1525448066.git.johannes.schinde...@gmx.de/ Ciao, Dscho
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
Hi, On Fri, May 4, 2018 at 9:21 AM, Elijah Newrenwrote: > On Fri, May 4, 2018 at 8:34 AM, Johannes Schindelin > wrote: >> The incredibly useful `git-tbdiff` tool to compare patch series (say, to see >> what changed between two iterations sent to the Git mailing list) is slightly >> less useful for this developer due to the fact that it requires the >> `hungarian` >> and `numpy` Python packages which are for some reason really hard to build in >> MSYS2. So hard that I even had to give up, because it was simply easier to >> reimplement the whole shebang as a builtin command. > > tbdiff is awesome; thanks for bringing it in as a builtin to git. > > I've run through a few cases, comparing output of tbdiff and > branch-diff. So far, what I've noted is that they produce largely the > same output except that: > > - tbdiff seems to shorten shas to 7 characters, branch-diff is using > 10, in git.git at least. (Probably a good change) Sorry, a quick self-correction here: tbdiff, when using an actual shortened sha, uses 10 characters. But when a patch doesn't have a match, tbdiff seems to use seven dashes on one side in lieu of a shortened sha, whereas branch-diff will use 10 characters whether it has an actual shortened sha or is just putting a bunch of dashes there. So, this is definitely a good change. > - tbdiff aligned output columns better when there were more than 9 > patches (I'll comment more on patch 09/18) > - As noted elsewhere in the review of round 1, tbdiff uses difflib > while branch-diff uses xdiff. I found some cases where that mattered, > and in all of them, I either felt like the difference was irrelevant > or that difflib was suboptimal, so this is definitely an improvement > for me. > - branch-diff produces it's output faster, and it is automatically > paged. This is really cool. > > Also, I don't have bash-completion for either tbdiff or branch-diff. > :-( But I saw some discussion on the v1 patches about how this gets > handled... :-)
Re: [PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
On Fri, May 4, 2018 at 8:34 AM, Johannes Schindelinwrote: > The incredibly useful `git-tbdiff` tool to compare patch series (say, to see > what changed between two iterations sent to the Git mailing list) is slightly > less useful for this developer due to the fact that it requires the > `hungarian` > and `numpy` Python packages which are for some reason really hard to build in > MSYS2. So hard that I even had to give up, because it was simply easier to > reimplement the whole shebang as a builtin command. tbdiff is awesome; thanks for bringing it in as a builtin to git. I've run through a few cases, comparing output of tbdiff and branch-diff. So far, what I've noted is that they produce largely the same output except that: - tbdiff seems to shorten shas to 7 characters, branch-diff is using 10, in git.git at least. (Probably a good change) - tbdiff aligned output columns better when there were more than 9 patches (I'll comment more on patch 09/18) - As noted elsewhere in the review of round 1, tbdiff uses difflib while branch-diff uses xdiff. I found some cases where that mattered, and in all of them, I either felt like the difference was irrelevant or that difflib was suboptimal, so this is definitely an improvement for me. - branch-diff produces it's output faster, and it is automatically paged. This is really cool. Also, I don't have bash-completion for either tbdiff or branch-diff. :-( But I saw some discussion on the v1 patches about how this gets handled... :-) Elijah
[PATCH v2 00/18] Add `branch-diff`, a `tbdiff` lookalike
The incredibly useful `git-tbdiff` tool to compare patch series (say, to see what changed between two iterations sent to the Git mailing list) is slightly less useful for this developer due to the fact that it requires the `hungarian` and `numpy` Python packages which are for some reason really hard to build in MSYS2. So hard that I even had to give up, because it was simply easier to reimplement the whole shebang as a builtin command. The project at https://github.com/trast/tbdiff seems to be dormant, anyway. Funny (and true) story: I looked at the open Pull Requests to see how active that project is, only to find to my surprise that I had submitted one in August 2015, and that it was still unanswered let alone merged. While at it, I forward-ported AEvar's patch to force `--decorate=no` because `git -p tbdiff` would fail otherwise. Side note: I work on implementing branch-diff not only to make life easier for reviewers who have to suffer through v2, v3, ... of my patch series, but also to verify my changes before submitting a new iteraion. And also, maybe even more importantly, I plan to use it to verify my merging-rebases of Git for Windows (for which I previously used to redirect the pre-rebase/post-rebase diffs vs upstream and then compare them using `git diff --no-index`). And of course any interested person can see what changes were necessary e.g. in the merging-rebase of Git for Windows onto v2.17.0 by running a command like: base=^{/Start.the.merging-rebase} tag=v2.17.0.windows.1 pre=$tag$base^2 git branch-diff --dual-color $pre$base..$pre $tag$base..$tag The --dual-color mode will identify the many changes that are solely due to different diff context lines (where otherwise uncolored lines start with a background-colored -/+ marker), i.e. merge conflicts I had to resolve. Changes since v1: - Fixed the usage to *not* say "rebase--helper" (oops, now everybody knows that I copy-edited that code... ;-)) - Removed `branch-diff` from the `git help` output for now, by removing the `info` keyword from the respective line in command-list.txt. - Removed the bogus `COLOR_DUAL_MODE` constant that was introduced in one patch, only to be removed in the next one. - Fixed typo "emtpy". - Fixed `make sparse` warnings. - Changed the usage string to avoid the confusing lower-case bare `base`. - Fixed tyop in commit message: "Pythong". - Removed an awkward empty line before a `continue;` statement. - Released the `line` strbuf after use. - Fixed a typo and some "foreigner's English" in the commit message of "branch-diff: also show the diff between patches". - Avoided introducing --no-patches too early and then replacing it by the version that uses the diff_options. - Fixed the man page to continue with upper-case after a colon. The branch-diff of v1 vs 2 is best viewed with --dual-color; This helps e.g. with identifying changed *context* lines in the diff: git branch-diff --dual-color origin/master branch-diff-v1 branch-diff-v2 (assuming that your `origin` points to git.git and that you fetched the tags from https://github.com/dscho/git). For example, you will see that the only change in patch #10 is a change in the diff context (due to the change of the usage string in patch #2): 10: 9810869ced9 ! 10: 2695a6abc46 branch-diff: do not show "function names" in hunk headers @@ -17,7 +17,7 @@ +#include "userdiff.h" static const char * const builtin_branch_diff_usage[] = { - N_("git rebase--helper [] ( A..B C..D | A...B | base A B )"), + N_("git branch-diff [] .. .."), @@ return data; } Johannes Schindelin (17): Add a function to solve least-cost assignment problems Add a new builtin: branch-diff branch-diff: first rudimentary implementation branch-diff: improve the order of the shown commits branch-diff: also show the diff between patches branch-diff: right-trim commit messages branch-diff: indent the diffs just like tbdiff branch-diff: suppress the diff headers branch-diff: adjust the output of the commit pairs branch-diff: do not show "function names" in hunk headers branch-diff: use color for the commit pairs color: provide inverted colors, too diff: add an internal option to dual-color diffs of diffs branch-diff: offer to dual-color the diffs branch-diff --dual-color: work around bogus white-space warning branch-diff: add a man page completion: support branch-diff Thomas Rast (1): branch-diff: add tests .gitignore | 1 + Documentation/git-branch-diff.txt | 239 ++ Makefile | 2 + builtin.h | 1 + builtin/branch-diff.c | 534 ++ color.h| 6 + command-list.txt | 1 + contrib/completion/git-completion.bash | 18 + diff.c | 76 +++-