Re: [Intel-gfx] [PATCH] dim: Add range-diff convenience wrapper

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Oct 05, 2018 at 11:43:58AM -0700, Lucas De Marchi wrote:
> On Thu, Oct 4, 2018 at 4:00 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > range-diff is awesome, but the interface is a bit silly. Add a bunch
> > of shortcuts, inspired by what git diff does.
> >
> > v2: Add it to the developer commmands list. With this dim range-diff
> > is useable on any git repo, not just a dim managed one.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  dim     | 14 ++++++++++++++
> >  dim.rst |  8 ++++++++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/dim b/dim
> > index 12c80e2051b6..f8079aa2dc4d 100755
> > --- a/dim
> > +++ b/dim
> > @@ -475,6 +475,19 @@ function dim_retip
> >         git rebase --onto $new_upstream $upstream $branch "$@"
> >  }
> >
> > +function dim_range_diff
> > +{
> > +       local branch
> > +
> > +       branch=${1:-@\{1\}}
> > +
> > +       if [[ $branch != "" && $(git rev-parse $branch | wc -l) -eq 1 ]] ; then
> 
> branch is never empty since you are providing a default.

Yeah that's a leftover, at first I didn't have the @{1} default
> 
> This also seems to fail silently (actually displaying a wrong info) if
> one  does dim range-diff HEAD^

$ dim range-diff HEAD^
-:  ------------ > 1:  8506ae553cd3 drm/vblank: Remove old-style comments
$ dim cite HEAD
8506ae553cd3 ("drm/vblank: Remove old-style comments")

Does exactly what I expect it to do.

> It would sound more useful if falling back to git-tbdiff in case
> range-diff is not available. What I kind of dislike on wrappers is the
> inability to provide more arguments. One that would be useful here
> would be a --no-patch if one only wants to see status change, not the
> change itself.

Can do that easily, it's a tiny change. Not sure about git-tbdiff, since
that's not really useful for people who don't have it installed.

> I'm still getting used to the tool so I'm always explicit: I always
> type both  ranges to tell exactly what I want to compare.
> 
> git range-diff basecommit..origin/branchname basecommit..HEAD

Way too much typing for me :-)

Cheers, Daniel
> 
> Lucas De Marchi
> 
> > +               git range-diff $branch...HEAD
> > +       else
> > +               git range-diff "$@"
> > +       fi
> > +}
> > +
> >  # update for-linux-next and for-linux-next-fixes branches
> >  function update_linux_next # branch next next-fixes fixes
> >  {
> > @@ -2188,6 +2201,7 @@ function list_developer_commands
> >                 "cite"
> >                 "fixes"
> >                 "retip"
> > +               "range-diff"
> >                 "sparse"
> >                 "tc"
> >                 # help commands
> > diff --git a/dim.rst b/dim.rst
> > index b149fa39445e..9e41133aeb8d 100644
> > --- a/dim.rst
> > +++ b/dim.rst
> > @@ -95,6 +95,14 @@ retip [*branch*] [*git-rebase option* ...]
> >  Rebase the given local branch, current branch by default, onto drm-tip. Options
> >  after the branch will be passed to **git-rebase**.
> >
> > +range-diff [ *commit-ish* | *git-range-diff options* ]
> > +------------------------------------------------------
> > +
> > +Convenience wrapper around the git range-diff command which automatically
> > +compares against HEAD if you only specify a commit-ish. In all other cases
> > +forwards to git range-diff. Defaults to @{1}, which is very useful for reviewing
> > +rebases.
> > +
> >  COMMANDS FOR COMMITTERS AND MAINTAINERS
> >  =======================================
> >
> > --
> > 2.19.0.rc2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel