[PATCH] spelling: fixes from proofreading of spell checker issues

Mads Kiilerich mads at kiilerich.com
Sun Nov 2 19:02:07 CST 2014


On 11/02/2014 10:09 PM, Martin von Zweigbergk wrote:
> On Sun Nov 02 2014 at 7:36:24 AM Mads Kiilerich <mads at kiilerich.com 
> <mailto:mads at kiilerich.com>> wrote:
>
>     # HG changeset patch
>     # User Mads Kiilerich <madski at unity3d.com <mailto:madski at unity3d.com>>
>     # Date 1397767658 -7200
>     #      Thu Apr 17 22:47:38 2014 +0200
>     # Branch stable
>     # Node ID 2ff580fd43fabecd3c0121ca0b8df4beb7100c8e
>     # Parent  cc1cbb0bba8ed1d95c8f1b8e27d4d2893e0dcca7
>     spelling: fixes from proofreading of spell checker issues
>
>     diff --git a/contrib/check-code.py b/contrib/check-code.py
>     --- a/contrib/check-code.py
>     +++ b/contrib/check-code.py
>     @@ -291,7 +291,7 @@ pypats = [
>           "always assign an opened file to a variable, and close it
>     afterwards"),
>          (r'[\s\(](open|file)\([^)]*\)\.',
>           "always assign an opened file to a variable, and close it
>     afterwards"),
>     -    (r'(?i)descendent', "the proper spelling is descendAnt"),
>     +    (r'(?i)descend[e]nt', "the proper spelling is descendAnt"),
>
>
> Is this a regular expression? If it is, I suspect this change was not 
> intentional.

It is intentional, to make sure the incorrect spelling doesn't occur in 
the source. It makes it unambiguous whether the wrong spelling should 
occur or not. As you can see elsewhere, the wrong spelling occurred 
elsewhere.

Anyway, this and other changes that your or someone else might disagree 
with can be left out when other parts are applied. Disputes can be 
resolved later.

>
>     diff --git a/hgext/largefiles/lfcommands.py
>     b/hgext/largefiles/lfcommands.py
>     --- a/hgext/largefiles/lfcommands.py
>     +++ b/hgext/largefiles/lfcommands.py
>     @@ -462,10 +462,10 @@ def updatelfiles(ui, repo, filelist=None
>                           expecthash != lfutil.hashfile(abslfile))):
>                          if lfile not in repo[None]: # not switched to
>     normal file
>                              util.unlinkpath(abslfile, ignoremissing=True)
>     -                    # use normallookup() to allocate entry in
>     largefiles
>     +                    # use normallookup() to allocate an entry in
>     largefiles
>                          # dirstate, because lack of it misleads
>                          # lfilesrepo.status() into recognition that
>     such cache
>
>
> If you're changing here anyway, you might as well fix the above as 
> well. The "misleads... into recognition" sounds odd to me. How about 
> s/recognition/thinking/?

Good point. (Here I mainly wanted to fix the odd REMOVED that seemed odd 
to the spell checker. I did not fix all the other things that could be 
fixed here. I will consider these comments in next round.)

>     -                    # missing files are REMOVED.
>     +                    # missing files has been removed.
>
>
> s/has/have/
>
> Also, "such cache missing files" seems clearer as "such missing files 
> in [from?] the cache" or "such files missing from the cache".
>
>                          lfdirstate.normallookup(lfile)
>                          update[lfile] = expecthash
>                  else:
>     diff --git a/hgext/largefiles/overrides.py
>     b/hgext/largefiles/overrides.py
>     --- a/hgext/largefiles/overrides.py
>     +++ b/hgext/largefiles/overrides.py
>     @@ -63,10 +63,10 @@ def installmatchandpatsfn(f):
>
>      def restorematchandpatsfn():
>          '''restores scmutil.matchandpats to what it was before
>     -    installnormalfilesmatchandpatsfn was called.  no-op if
>     scmutil.matchandpats
>     +    installmatchandpatsfn was called. No-op if scmutil.matchandpats
>          is its original function.
>
>     -    Note that n calls to installnormalfilesmatchandpatsfn will
>     require n calls
>     +    Note that n calls to installmatchandpatsfn will require n calls
>          to restore matchfn to reverse'''
>
>
> Drop "to reverse"? It seems like "to restore" already says that.

Ack.

>     diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>     --- a/mercurial/cmdutil.py
>     +++ b/mercurial/cmdutil.py
>     @@ -113,7 +113,7 @@ def logmessage(ui, opts):
>      def mergeeditform(ctxorbool, baseform):
>          """build appropriate editform from ctxorbool and baseform
>
>     -    'cxtorbool' is one of a ctx to be committed, or a bool whether
>     +    'ctxorbool' is one of a ctx to be committed, or a bool whether
>          merging is committed.
>
>
> I'd say "..is either a ctx to be commited, or a bool indicating 
> whether merging is committed." Also, should it be "whether the merge 
> has been committed"? Or "whether merging is permitted"?
>
> At this point in the review, I noticed that the patch has already been 
> applied. Let me know if I should continue reviewing.

It has? Where?

If so, further comments would still be appreciated, preferably as 
patches, but something I can pick up for another round of patches could 
also work. But again, my goal here is mainly to address the issues a 
spell checker points out, not to fix all issues ;-) .

/Mads
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20141103/6d62fa95/attachment.html>


More information about the Mercurial-devel mailing list