Patchwork D5490: commit: remove ignore whitespace option on --interactive (issue6042)

login
register
mail settings
Submitter phabricator
Date Jan. 11, 2019, 3:12 p.m.
Message ID <490066719941edc96e3a031d231b9de4@localhost.localdomain>
Download mbox | patch
Permalink /patch/37676/
State Not Applicable
Headers show

Comments

phabricator - Jan. 11, 2019, 3:12 p.m.
navaneeth.suresh updated this revision to Diff 13177.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5490?vs=13166&id=13177

REVISION DETAIL
  https://phab.mercurial-scm.org/D5490

AFFECTED FILES
  hgext/record.py
  mercurial/cmdutil.py
  tests/test-commit-interactive.t

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: yuja, durin42, pulkit, mercurial-devel
Yuya Nishihara - Jan. 13, 2019, 1:47 a.m.
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -237,6 +237,7 @@
>  def dorecord(ui, repo, commitfunc, cmdsuggest, backupall,
>              filterfn, *pats, **opts):
>      opts = pycompat.byteskwargs(opts)
> +    ignorews = opts.get('ignorews', False)

It isn't nice to mix command options and internal flags. Instead, maybe we can
first change `record()` to not call `commands.commit()`, and pass in `whitespace`
option to `cmdutil.dorecord()`.

```
def record(ui, repo, *pats, **opts):
    opts = pycompat.byteskwargs(opts)
    ...
    with repo.wlock(), repo.lock():
        ret = cmdutil.dorecord(ui, repo, commands.commit, ..., whitespace=True,
                               pats, opts)
        ...
```

And I don't think `ignorews` is good name. It doesn't mean whitespace is
ignored.

>  def qrefresh(origfn, ui, repo, *pats, **opts):
>      if not opts[r'interactive']:
> @@ -89,7 +89,7 @@
>  
>      # backup all changed files
>      cmdutil.dorecord(ui, repo, committomq, None, True,
> -                    cmdutil.recordfilter, *pats, **opts)
> +                    cmdutil.recordfilter, *pats, ignorews=True, **opts)

Perhaps, this one should be `ignorefs=False` since qrefresh has no diffwsopts.
phabricator - Jan. 13, 2019, 1:55 a.m.
yuja added a comment.


  > - a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -237,6 +237,7 @@ def dorecord(ui, repo, commitfunc, cmdsuggest, backupall, filterfn, *pats, **opts): opts = pycompat.byteskwargs(opts) +    ignorews = opts.get('ignorews', False)
  
  It isn't nice to mix command options and internal flags. Instead, maybe we can
  first change `record()` to not call `commands.commit()`, and pass in `whitespace`
  option to `cmdutil.dorecord()`.
  
    def record(ui, repo, *pats, **opts):
        opts = pycompat.byteskwargs(opts)
        ...
        with repo.wlock(), repo.lock():
            ret = cmdutil.dorecord(ui, repo, commands.commit, ..., whitespace=True,
                                   pats, opts)
            ...
  
  And I don't think `ignorews` is good name. It doesn't mean whitespace is
  ignored.
  
  >   def qrefresh(origfn, ui, repo, *pats, **opts):
  >       if not opts[r'interactive']:
  > 
  > @@ -89,7 +89,7 @@
  > 
  >   1. backup all changed files cmdutil.dorecord(ui, repo, committomq, None, True,
  > - cmdutil.recordfilter, *pats, **opts) +                    cmdutil.recordfilter, *pats, ignorews=True, **opts)
  
  Perhaps, this one should be `ignorefs=False` since qrefresh has no diffwsopts.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5490

To: navaneeth.suresh, #hg-reviewers
Cc: yuja, durin42, pulkit, mercurial-devel

Patch

diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t
--- a/tests/test-commit-interactive.t
+++ b/tests/test-commit-interactive.t
@@ -1807,3 +1807,40 @@ 
   n   0         -1 unset               subdir/f1
   $ hg status -A subdir/f1
   M subdir/f1
+
+making --interactive not ignore whitespaces with the following hgrc:
+[diff]
+ignorews=True
+  $ hg init issue6042
+  $ cd issue6042
+  $ cat >> $HGRCPATH << EOF
+  > [diff]
+  > ignorews = True
+  > [extensions]
+  > record =
+  > EOF
+  $ echo a > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'a ' > a
+  $ hg diff
+  $ hg commit -i -m 'add ws to a' <<EOF
+  > y
+  > y
+  > n
+  > EOF
+  diff --git a/a b/a
+  1 hunks, 1 lines changed
+  examine changes to 'a'? [Ynesfdaq?] y
+  
+  @@ -1,1 +1,1 @@
+  -a
+  +a 
+  record this change to 'a'? [Ynesfdaq?] y
+  
+
+let's check whether record extension works fine or not after the fix
+  $ echo 'a  ' > a
+  $ hg record --ignore-all-space
+  no changes to record
+  [1]
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -237,6 +237,7 @@ 
 def dorecord(ui, repo, commitfunc, cmdsuggest, backupall,
             filterfn, *pats, **opts):
     opts = pycompat.byteskwargs(opts)
+    ignorews = opts.get('ignorews', False)
     if not ui.interactive():
         if cmdsuggest:
             msg = _('running non-interactively, use %s instead') % cmdsuggest
@@ -282,7 +283,7 @@ 
         status = repo.status(match=match)
         if not force:
             repo.checkcommitpatterns(wctx, vdirs, match, status, fail)
-        diffopts = patch.difffeatureopts(ui, opts=opts, whitespace=True)
+        diffopts = patch.difffeatureopts(ui, opts=opts, whitespace=ignorews)
         diffopts.nodates = True
         diffopts.git = True
         diffopts.showfunc = True
diff --git a/hgext/record.py b/hgext/record.py
--- a/hgext/record.py
+++ b/hgext/record.py
@@ -72,7 +72,7 @@ 
     opts[r"interactive"] = True
     overrides = {('experimental', 'crecord'): False}
     with ui.configoverride(overrides, 'record'):
-        return commands.commit(ui, repo, *pats, **opts)
+        return commands.commit(ui, repo, *pats, ignorews=True, **opts)
 
 def qrefresh(origfn, ui, repo, *pats, **opts):
     if not opts[r'interactive']:
@@ -89,7 +89,7 @@ 
 
     # backup all changed files
     cmdutil.dorecord(ui, repo, committomq, None, True,
-                    cmdutil.recordfilter, *pats, **opts)
+                    cmdutil.recordfilter, *pats, ignorews=True, **opts)
 
 # This command registration is replaced during uisetup().
 @command('qrecord',
@@ -120,7 +120,7 @@ 
     overrides = {('experimental', 'crecord'): False}
     with ui.configoverride(overrides, 'record'):
         cmdutil.dorecord(ui, repo, committomq, cmdsuggest, False,
-                         cmdutil.recordfilter, *pats, **opts)
+                         cmdutil.recordfilter, *pats, ignorews=True, **opts)
 
 def qnew(origfn, ui, repo, patch, *args, **opts):
     if opts[r'interactive']: