stgit: config option for diff-opts

stgit: config option for diff-opts

[Jon Smirl]

Could I get a config option added to stgit for diff-opts? So that I
can always have git diff -M set on renames.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: stgit: config option for diff-opts

[Karl Hasselström]

On 2008-01-22 22:04:21 -0500, Jon Smirl wrote:

> Could I get a config option added to stgit for diff-opts? So that I
> can always have git diff -M set on renames.

Good idea. I've often needed this myself, without consciously
realizing it -- my bash is set to save an infinite amount of history,
so Ctrl+R always helps me out ...

Will whip up, unless someone beats me to it. It's getting sorta late,
so I won't have time for at least a day or two.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: stgit: config option for diff-opts

[Yann Dirson]

On Wed, Jan 23, 2008 at 04:18:29AM +0100, Karl Hasselström wrote:
> On 2008-01-22 22:04:21 -0500, Jon Smirl wrote:
> 
> > Could I get a config option added to stgit for diff-opts? So that I
> > can always have git diff -M set on renames.
> 
> Good idea. I've often needed this myself, without consciously
> realizing it -- my bash is set to save an infinite amount of history,
> so Ctrl+R always helps me out ...
> 
> Will whip up, unless someone beats me to it. It's getting sorta late,
> so I won't have time for at least a day or two.

I also think that some config stuff should be done, but I could not
come with a usable design.

See eg. http://marc.info/?l=git&m=118102609623481&w=4

Another problem not explicitely mentionned there is that we my want on
the same project to have different settings, eg. for viewing (as
compact as possible), exporting (GNU compatibility), and feeding to
other git commands (with --binary).

Best regards,
-- 
Yann

[StGit PATCH 0/4] --diff-opts in config file

[Karl Hasselström]

This is first two cleanup patches, followed by one patch that makes
"stg show" handle diff options like the other commands, and last a
patch that reads diff opts from the config file.

The first two patches are in kha/safe, but the latter two are in
kha/experimental because -- as Yann pointed out -- it's good to make
sure the design is OK before committing to it.

I've solved the problem of overriding the config on the command line
by making --diff-opts='--foo --bar' _add_ options, and special-cased
--diff-opts='' to clear all options, including those from the config
file, since appending no options makes no sense.

We'll still have to consider if and how to support different options
for different tools.

Oh, and Jon: is this what you wanted, by the way? :-)

---

Karl Hasselström (4):
      Read default diff options from the user's config
      Let "stg show" use the unified --diff-opts handling
      Refactor --diff-opts handling
      Remove unused default values


 examples/gitconfig       |    4 ++++
 stgit/commands/diff.py   |   14 ++++----------
 stgit/commands/edit.py   |   14 +++-----------
 stgit/commands/export.py |   12 +++---------
 stgit/commands/files.py  |   13 ++++---------
 stgit/commands/mail.py   |   12 +++---------
 stgit/commands/show.py   |   13 ++++---------
 stgit/commands/status.py |   17 +++++------------
 stgit/utils.py           |   13 +++++++++++++
 9 files changed, 43 insertions(+), 69 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH 1/4] Remove unused default values

[Karl Hasselström]

This function was called from only one place, and the default values
weren't used there, so they're useless.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/status.py |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)


diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index 708dd16..02a5832 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -66,9 +66,8 @@ options = [make_option('-m', '--modified',
                        action = 'store_true')]
 
 
-def status(files = None, modified = False, new = False, deleted = False,
-           conflict = False, unknown = False, noexclude = False,
-           diff_flags = []):
+def status(files, modified, new, deleted, conflict, unknown, noexclude,
+           diff_flags):
     """Show the tree status
     """
     cache_files = git.tree_status(files,

[StGit PATCH 2/4] Refactor --diff-opts handling

[Karl Hasselström]

Lots of commands take a -O/--diff-opts flag, and they all handle it
identically. So break that out into a library function.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/diff.py   |   14 ++++----------
 stgit/commands/edit.py   |   14 +++-----------
 stgit/commands/export.py |   12 +++---------
 stgit/commands/files.py  |   13 ++++---------
 stgit/commands/mail.py   |   12 +++---------
 stgit/commands/status.py |   12 +++---------
 stgit/utils.py           |   12 ++++++++++++
 7 files changed, 32 insertions(+), 57 deletions(-)


diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index 1425518..7c213d1 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -46,12 +46,10 @@ directory = DirectoryHasRepository()
 options = [make_option('-r', '--range',
                        metavar = 'rev1[..[rev2]]', dest = 'revs',
                        help = 'show the diff between revisions'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('-s', '--stat',
                        help = 'show the stat instead of the diff',
-                       action = 'store_true')]
-
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 def func(parser, options, args):
     """Show the tree diff
@@ -83,16 +81,12 @@ def func(parser, options, args):
         rev1 = 'HEAD'
         rev2 = None
 
-    if options.diff_opts:
-        diff_flags = options.diff_opts.split()
-    else:
-        diff_flags = []
-
     if options.stat:
         out.stdout_raw(git.diffstat(args, git_id(crt_series, rev1),
                                     git_id(crt_series, rev2)) + '\n')
     else:
         diff_str = git.diff(args, git_id(crt_series, rev1),
-                            git_id(crt_series, rev2), diff_flags = diff_flags )
+                            git_id(crt_series, rev2),
+                            diff_flags = options.diff_flags)
         if diff_str:
             pager(diff_str)
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index 2e8ae37..da67275 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -61,8 +61,6 @@ directory = DirectoryGotoToplevel()
 options = [make_option('-d', '--diff',
                        help = 'edit the patch diff',
                        action = 'store_true'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('--undo',
                        help = 'revert the commit generated by the last edit',
                        action = 'store_true'),
@@ -80,7 +78,8 @@ options = [make_option('-d', '--diff',
                        help = 'replace the committer name with COMMNAME'),
            make_option('--commemail',
                        help = 'replace the committer e-mail with COMMEMAIL')
-           ] + make_sign_options() + make_message_options()
+           ] + (make_sign_options() + make_message_options()
+                + make_diff_opts_option())
 
 def __update_patch(pname, text, options):
     """Update the current patch from the given text.
@@ -130,13 +129,6 @@ def __generate_file(pname, write_fn, options):
     """
     patch = crt_series.get_patch(pname)
 
-    if options.diff_opts:
-        if not options.diff:
-            raise CmdException, '--diff-opts only available with --diff'
-        diff_flags = options.diff_opts.split()
-    else:
-        diff_flags = []
-
     # generate the file to be edited
     descr = patch.get_description().strip()
     authdate = patch.get_authdate()
@@ -164,7 +156,7 @@ def __generate_file(pname, write_fn, options):
 
         tmpl_dict['diffstat'] = git.diffstat(rev1 = bottom, rev2 = top)
         tmpl_dict['diff'] = git.diff(rev1 = bottom, rev2 = top,
-                                     diff_flags = diff_flags)
+                                     diff_flags = options.diff_flags)
 
     for key in tmpl_dict:
         # make empty strings if key is not available
diff --git a/stgit/commands/export.py b/stgit/commands/export.py
index 9131729..16c64ba 100644
--- a/stgit/commands/export.py
+++ b/stgit/commands/export.py
@@ -64,11 +64,10 @@ options = [make_option('-d', '--dir',
                        help = 'Use FILE as a template'),
            make_option('-b', '--branch',
                        help = 'use BRANCH instead of the default one'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('-s', '--stdout',
                        help = 'dump the patches to the standard output',
-                       action = 'store_true')]
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 
 def func(parser, options, args):
@@ -89,11 +88,6 @@ def func(parser, options, args):
             os.makedirs(dirname)
         series = file(os.path.join(dirname, 'series'), 'w+')
 
-    if options.diff_opts:
-        diff_flags = options.diff_opts.split()
-    else:
-        diff_flags = []
-
     applied = crt_series.get_applied()
     if len(args) != 0:
         patches = parse_patches(args, applied)
@@ -180,7 +174,7 @@ def func(parser, options, args):
         f.write(descr)
         f.write(git.diff(rev1 = patch.get_bottom(),
                          rev2 = patch.get_top(),
-                         diff_flags = diff_flags))
+                         diff_flags = options.diff_flags))
         if not options.stdout:
             f.close()
         patch_no += 1
diff --git a/stgit/commands/files.py b/stgit/commands/files.py
index 4550251..ab1f6a3 100644
--- a/stgit/commands/files.py
+++ b/stgit/commands/files.py
@@ -40,11 +40,10 @@ options = [make_option('-s', '--stat',
                        action = 'store_true'),
            make_option('-b', '--branch',
                        help = 'use BRANCH instead of the default one'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('--bare',
                        help = 'bare file names (useful for scripting)',
-                       action = 'store_true')]
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 
 def func(parser, options, args):
@@ -65,9 +64,5 @@ def func(parser, options, args):
     elif options.bare:
         out.stdout_raw(git.barefiles(rev1, rev2) + '\n')
     else:
-        if options.diff_opts:
-            diff_flags = options.diff_opts.split()
-        else:
-            diff_flags = []
-
-        out.stdout_raw(git.files(rev1, rev2, diff_flags = diff_flags) + '\n')
+        out.stdout_raw(git.files(rev1, rev2, diff_flags = options.diff_flags)
+                       + '\n')
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index eea84f2..4aa16fb 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -144,11 +144,10 @@ options = [make_option('-a', '--all',
                        action = 'store_true'),
            make_option('-b', '--branch',
                        help = 'use BRANCH instead of the default one'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('-m', '--mbox',
                        help = 'generate an mbox file instead of sending',
-                       action = 'store_true')]
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 
 def __get_sender():
@@ -426,11 +425,6 @@ def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
             prefix_str = confprefix + ' '
         else:
             prefix_str = ''
-        
-    if options.diff_opts:
-        diff_flags = options.diff_opts.split()
-    else:
-        diff_flags = []
 
     total_nr_str = str(total_nr)
     patch_nr_str = str(patch_nr).zfill(len(total_nr_str))
@@ -450,7 +444,7 @@ def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
                  'diff':         git.diff(
                      rev1 = git_id(crt_series, '%s//bottom' % patch),
                      rev2 = git_id(crt_series, '%s//top' % patch),
-                     diff_flags = diff_flags),
+                     diff_flags = options.diff_flags),
                  'diffstat':     git.diffstat(
                      rev1 = git_id(crt_series, '%s//bottom'%patch),
                      rev2 = git_id(crt_series, '%s//top' % patch)),
diff --git a/stgit/commands/status.py b/stgit/commands/status.py
index 02a5832..6da4516 100644
--- a/stgit/commands/status.py
+++ b/stgit/commands/status.py
@@ -59,11 +59,10 @@ options = [make_option('-m', '--modified',
            make_option('-x', '--noexclude',
                        help = 'do not exclude any files from listing',
                        action = 'store_true'),
-           make_option('-O', '--diff-opts',
-                       help = 'options to pass to git-diff'),
            make_option('--reset',
                        help = 'reset the current tree changes',
-                       action = 'store_true')]
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 
 def status(files, modified, new, deleted, conflict, unknown, noexclude,
@@ -115,11 +114,6 @@ def func(parser, options, args):
             resolved_all()
             git.reset()
     else:
-        if options.diff_opts:
-            diff_flags = options.diff_opts.split()
-        else:
-            diff_flags = []
-
         status(args, options.modified, options.new, options.deleted,
                options.conflict, options.unknown, options.noexclude,
-               diff_flags = diff_flags)
+               options.diff_flags)
diff --git a/stgit/utils.py b/stgit/utils.py
index 2ff1d74..00776b0 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -313,6 +313,18 @@ def make_message_options():
               metavar = 'FILE', dest = 'save_template', type = 'string',
               help = 'save the message template to FILE and exit')]
 
+def make_diff_opts_option():
+    def diff_opts_callback(option, opt_str, value, parser):
+        if value:
+            parser.values.diff_flags.extend(value.split())
+        else:
+            parser.values.diff_flags = []
+    return [optparse.make_option(
+        '-O', '--diff-opts', dest = 'diff_flags', default = [],
+        action = 'callback', callback = diff_opts_callback,
+        type = 'string', metavar = 'OPTIONS',
+        help = 'extra options to pass to "git diff"')]
+
 # Exit codes.
 STGIT_SUCCESS = 0        # everything's OK
 STGIT_GENERAL_ERROR = 1  # seems to be non-command-specific error

[StGit PATCH 3/4] Let "stg show" use the unified --diff-opts handling

[Karl Hasselström]

This introduces a small UI change: "stg show" called that flag
--show-opts. This could of course be avoided, but I don't think it's
worth it, since git-diff and git-show accept mostly the same options.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

Catalin, do you agree?

 stgit/commands/show.py |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)


diff --git a/stgit/commands/show.py b/stgit/commands/show.py
index 72d1be3..b77a9c8 100644
--- a/stgit/commands/show.py
+++ b/stgit/commands/show.py
@@ -38,9 +38,8 @@ options = [make_option('-b', '--branch',
                        action = 'store_true'),
            make_option('-u', '--unapplied',
                        help = 'show the unapplied patches',
-                       action = 'store_true'),
-           make_option('-O', '--show-opts',
-                       help = 'options to pass to "git show"')]
+                       action = 'store_true')
+           ] + make_diff_opts_option()
 
 
 def func(parser, options, args):
@@ -62,13 +61,9 @@ def func(parser, options, args):
             patches = parse_patches(args, applied + unapplied + \
                                     crt_series.get_hidden(), len(applied))
 
-    if options.show_opts:
-        show_flags = options.show_opts.split()
-    else:
-        show_flags = []
-
     commit_ids = [git_id(crt_series, patch) for patch in patches]
-    commit_str = '\n'.join([git.pretty_commit(commit_id, flags = show_flags)
+    commit_str = '\n'.join([git.pretty_commit(commit_id,
+                                              flags = options.diff_flags)
                             for commit_id in commit_ids])
     if commit_str:
         pager(commit_str)

[StGit PATCH 4/4] Read default diff options from the user's config

[Karl Hasselström]

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 examples/gitconfig |    4 ++++
 stgit/utils.py     |    3 ++-
 2 files changed, 6 insertions(+), 1 deletions(-)


diff --git a/examples/gitconfig b/examples/gitconfig
index cd9b569..c16f786 100644
--- a/examples/gitconfig
+++ b/examples/gitconfig
@@ -95,6 +95,10 @@
 	# The maximum length of an automatically generated patch name
 	#namelenth = 30
 
+	# Extra options to pass to "git diff" (extend/override with
+	# -O/--diff-opts). For example, -M turns on rename detection.
+	#diff-opts = -M
+
 [mail "alias"]
 	# E-mail aliases used with the 'mail' command
 	git = git@vger.kernel.org
diff --git a/stgit/utils.py b/stgit/utils.py
index 00776b0..69203dc 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -320,7 +320,8 @@ def make_diff_opts_option():
         else:
             parser.values.diff_flags = []
     return [optparse.make_option(
-        '-O', '--diff-opts', dest = 'diff_flags', default = [],
+        '-O', '--diff-opts', dest = 'diff_flags',
+        default = (config.get('stgit.diff-opts') or '').split(),
         action = 'callback', callback = diff_opts_callback,
         type = 'string', metavar = 'OPTIONS',
         help = 'extra options to pass to "git diff"')]

Re: [StGit PATCH 0/4] --diff-opts in config file

[Karl Hasselström]

On 2008-01-24 09:06:54 +0100, Karl Hasselström wrote:

> I've solved the problem of overriding the config on the command line
> by making --diff-opts='--foo --bar' _add_ options, and special-cased
> --diff-opts='' to clear all options, including those from the config
> file, since appending no options makes no sense.

Mph. I just realized that none of the patches add any documentation
that explains this.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGit PATCH 0/4] --diff-opts in config file

[Jon Smirl]

On 1/24/08, Karl Hasselström <kha@treskal.com> wrote:
> This is first two cleanup patches, followed by one patch that makes
> "stg show" handle diff options like the other commands, and last a
> patch that reads diff opts from the config file.
>
> The first two patches are in kha/safe, but the latter two are in
> kha/experimental because -- as Yann pointed out -- it's good to make
> sure the design is OK before committing to it.
>
> I've solved the problem of overriding the config on the command line
> by making --diff-opts='--foo --bar' _add_ options, and special-cased
> --diff-opts='' to clear all options, including those from the config
> file, since appending no options makes no sense.
>
> We'll still have to consider if and how to support different options
> for different tools.
>
> Oh, and Jon: is this what you wanted, by the way? :-)

This patch series is fine for my uses.

>
> ---
>
> Karl Hasselström (4):
>       Read default diff options from the user's config
>       Let "stg show" use the unified --diff-opts handling
>       Refactor --diff-opts handling
>       Remove unused default values
>
>
>  examples/gitconfig       |    4 ++++
>  stgit/commands/diff.py   |   14 ++++----------
>  stgit/commands/edit.py   |   14 +++-----------
>  stgit/commands/export.py |   12 +++---------
>  stgit/commands/files.py  |   13 ++++---------
>  stgit/commands/mail.py   |   12 +++---------
>  stgit/commands/show.py   |   13 ++++---------
>  stgit/commands/status.py |   17 +++++------------
>  stgit/utils.py           |   13 +++++++++++++
>  9 files changed, 43 insertions(+), 69 deletions(-)
>
> --
> Karl Hasselström, kha@treskal.com
>       www.treskal.com/kalle
>


-- 
Jon Smirl
jonsmirl@gmail.com