Patchwork [1,of,3,stable] obsolete: fix language and grammer in module docstring

login
register
mail settings
Submitter Martin Geisler
Date April 19, 2014, 6:14 p.m.
Message ID <bb298bc11a31edccb815.1397931293@hbox.dyndns.org>
Download mbox | patch
Permalink /patch/4410/
State Deferred
Headers show

Comments

Martin Geisler - April 19, 2014, 6:14 p.m.
# HG changeset patch
# User Martin Geisler <martin@geisler.net>
# Date 1397929929 -7200
#      Sat Apr 19 19:52:09 2014 +0200
# Branch stable
# Node ID bb298bc11a31edccb815a270635f0ef31a5f3f44
# Parent  d924e387604f4a1b90469de773517841eba40c80
obsolete: fix language and grammer in module docstring
Siddharth Agarwal - April 20, 2014, 9:12 a.m.
On 04/19/2014 11:44 PM, Martin Geisler wrote:
> # HG changeset patch
> # User Martin Geisler <martin@geisler.net>
> # Date 1397929929 -7200
> #      Sat Apr 19 19:52:09 2014 +0200
> # Branch stable
> # Node ID bb298bc11a31edccb815a270635f0ef31a5f3f44
> # Parent  d924e387604f4a1b90469de773517841eba40c80
> obsolete: fix language and grammer in module docstring

Ironic? :)

Other than that, these look good to me.

>
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -6,7 +6,7 @@
>   # This software may be used and distributed according to the terms of the
>   # GNU General Public License version 2 or any later version.
>   
> -"""Obsolete markers handling
> +"""Obsolete marker handling
>   
>   An obsolete marker maps an old changeset to a list of new
>   changesets. If the list of new changesets is empty, the old changeset
> @@ -14,30 +14,30 @@
>   "replaced" by the new changesets.
>   
>   Obsolete markers can be used to record and distribute changeset graph
> -transformations performed by history rewriting operations, and help
> -building new tools to reconciliate conflicting rewriting actions. To
> -facilitate conflicts resolution, markers include various annotations
> +transformations performed by history rewrite operations, and help
> +building new tools to reconcile conflicting rewrite actions. To
> +facilitate conflict resolution, markers include various annotations
>   besides old and news changeset identifiers, such as creation date or
>   author name.
>   
> -The old obsoleted changeset is called "precursor" and possible replacements are
> -called "successors".  Markers that used changeset X as a precursors are called
> +The old obsoleted changeset is called a "precursor" and possible replacements are
> +called "successors".  Markers that used changeset X as a precursor are called
>   "successor markers of X" because they hold information about the successors of
>   X. Markers that use changeset Y as a successors are call "precursor markers of
>   Y" because they hold information about the precursors of Y.
>   
>   Examples:
>   
> -- When changeset A is replacement by a changeset A', one marker is stored:
> +- When changeset A is replaced by changeset A', one marker is stored:
>   
>       (A, (A'))
>   
> -- When changesets A and B are folded into a new changeset C two markers are
> +- When changesets A and B are folded into a new changeset C, two markers are
>     stored:
>   
>       (A, (C,)) and (B, (C,))
>   
> -- When changeset A is simply "pruned" from the graph, a marker in create:
> +- When changeset A is simply "pruned" from the graph, a marker is created:
>   
>       (A, ())
>   
> @@ -45,9 +45,9 @@
>   
>       (A, (C, C))
>   
> -  We use a single marker to distinct the "split" case from the "divergence"
> -  case. If two independents operation rewrite the same changeset A in to A' and
> -  A'' when have an error case: divergent rewriting. We can detect it because
> +  We use a single marker to distinguish the "split" case from the "divergence"
> +  case. If two independent operations rewrite the same changeset A in to A' and
> +  A'', we have an error case: divergent rewriting. We can detect it because
>     two markers will be created independently:
>   
>     (A, (B,)) and (A, (C,))
> @@ -65,12 +65,12 @@
>   
>   The header is followed by the markers. Each marker is made of:
>   
> -- 1 unsigned byte: number of new changesets "R", could be zero.
> +- 1 unsigned byte: number of new changesets "R", can be zero.
>   
>   - 1 unsigned 32-bits integer: metadata size "M" in bytes.
>   
> -- 1 byte: a bit field. It is reserved for flags used in obsolete
> -  markers common operations, to avoid repeated decoding of metadata
> +- 1 byte: a bit field. It is reserved for flags used in common
> +  obsolete marker operations, to avoid repeated decoding of metadata
>     entries.
>   
>   - 20 bytes: obsoleted changeset identifier.
> @@ -78,7 +78,7 @@
>   - N*20 bytes: new changesets identifiers.
>   
>   - M bytes: metadata as a sequence of nul-terminated strings. Each
> -  string contains a key and a value, separated by a color ':', without
> +  string contains a key and a value, separated by a colon ':', without
>     additional encoding. Keys cannot contain '\0' or ':' and values
>     cannot contain '\0'.
>   """
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Martin Geisler - April 21, 2014, 10:12 a.m.
Siddharth Agarwal <sid@less-broken.com> writes:

> On 04/19/2014 11:44 PM, Martin Geisler wrote:
>> # HG changeset patch
>> # User Martin Geisler <martin@geisler.net>
>> # Date 1397929929 -7200
>> #      Sat Apr 19 19:52:09 2014 +0200
>> # Branch stable
>> # Node ID bb298bc11a31edccb815a270635f0ef31a5f3f44
>> # Parent  d924e387604f4a1b90469de773517841eba40c80
>> obsolete: fix language and grammer in module docstring
>
> Ironic? :)

Ah, you must be talking about me not being able to spell "grammar"? I
wish I had been clever enough to do that ironically :)

I can resend with that fixed if you like? I would also like to wrap the
strings at a consistent width (72 chars is default in Emacs), but I'm
unsure if we want this on stable?

> Other than that, these look good to me.
>
>>
>> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
>> --- a/mercurial/obsolete.py
>> +++ b/mercurial/obsolete.py
>> @@ -6,7 +6,7 @@
>>   # This software may be used and distributed according to the terms of the
>>   # GNU General Public License version 2 or any later version.
>>   -"""Obsolete markers handling
>> +"""Obsolete marker handling
>>     An obsolete marker maps an old changeset to a list of new
>>   changesets. If the list of new changesets is empty, the old changeset
>> @@ -14,30 +14,30 @@
>>   "replaced" by the new changesets.
>>     Obsolete markers can be used to record and distribute changeset
>> graph
>> -transformations performed by history rewriting operations, and help
>> -building new tools to reconciliate conflicting rewriting actions. To
>> -facilitate conflicts resolution, markers include various annotations
>> +transformations performed by history rewrite operations, and help
>> +building new tools to reconcile conflicting rewrite actions. To
>> +facilitate conflict resolution, markers include various annotations

The above paragraph is wrapped at 72 chars. The paragraph below seems to
be wrapped at 80 chars:

>> -called "successors".  Markers that used changeset X as a precursors are called
>> +The old obsoleted changeset is called a "precursor" and possible replacements are
>> +called "successors".  Markers that used changeset X as a precursor are called
>>   "successor markers of X" because they hold information about the successors of
>>   X. Markers that use changeset Y as a successors are call "precursor markers of
>>   Y" because they hold information about the precursors of Y.
Mads Kiilerich - April 21, 2014, 10:57 a.m.
On 04/21/2014 12:12 PM, Martin Geisler wrote:
> I can resend with that fixed if you like? I would also like to wrap the
> strings at a consistent width (72 chars is default in Emacs), but I'm
> unsure if we want this on stable?

It do not seem to be a good idea to wrap it to 72 chars just because 
that is the default in one of the many editors people use for editing 
the source code. CodingStyle only mentions the 80 character limit. 
Reformatting to other conventions will just add unnecessary noise - 
especially when the next guy come by and want to wrap to 80 characters 
and has good arguments why that would be better.

/Mads
Matt Mackall - April 21, 2014, 11:10 a.m.
On Mon, 2014-04-21 at 12:12 +0200, Martin Geisler wrote:
> Siddharth Agarwal <sid@less-broken.com> writes:
> 
> > On 04/19/2014 11:44 PM, Martin Geisler wrote:
> >> # HG changeset patch
> >> # User Martin Geisler <martin@geisler.net>
> >> # Date 1397929929 -7200
> >> #      Sat Apr 19 19:52:09 2014 +0200
> >> # Branch stable
> >> # Node ID bb298bc11a31edccb815a270635f0ef31a5f3f44
> >> # Parent  d924e387604f4a1b90469de773517841eba40c80
> >> obsolete: fix language and grammer in module docstring
> >
> > Ironic? :)
> 
> Ah, you must be talking about me not being able to spell "grammar"? I
> wish I had been clever enough to do that ironically :)

> I can resend with that fixed if you like? I would also like to wrap the
> strings at a consistent width (72 chars is default in Emacs), but I'm
> unsure if we want this on stable?

I've already fixed both of these in flight.
Martin Geisler - April 21, 2014, 11:44 a.m.
Matt Mackall <mpm@selenic.com> writes:

> On Mon, 2014-04-21 at 12:12 +0200, Martin Geisler wrote:
>> Siddharth Agarwal <sid@less-broken.com> writes:
>> 
>> > On 04/19/2014 11:44 PM, Martin Geisler wrote:
>> >> # HG changeset patch
>> >> # User Martin Geisler <martin@geisler.net>
>> >> # Date 1397929929 -7200
>> >> #      Sat Apr 19 19:52:09 2014 +0200
>> >> # Branch stable
>> >> # Node ID bb298bc11a31edccb815a270635f0ef31a5f3f44
>> >> # Parent  d924e387604f4a1b90469de773517841eba40c80
>> >> obsolete: fix language and grammer in module docstring
>> >
>> > Ironic? :)
>> 
>> Ah, you must be talking about me not being able to spell "grammar"? I
>> wish I had been clever enough to do that ironically :)
>
>> I can resend with that fixed if you like? I would also like to wrap the
>> strings at a consistent width (72 chars is default in Emacs), but I'm
>> unsure if we want this on stable?
>
> I've already fixed both of these in flight.

Perfect, thanks!
Martin Geisler - April 21, 2014, 12:07 p.m.
Mads Kiilerich <mads@kiilerich.com> writes:

Hi Mads,

This is not super important and I agree that too much rewrapping
generates noise.

> On 04/21/2014 12:12 PM, Martin Geisler wrote:
>> I can resend with that fixed if you like? I would also like to wrap
>> the strings at a consistent width (72 chars is default in Emacs), but
>> I'm unsure if we want this on stable?
>
> It do not seem to be a good idea to wrap it to 72 chars just because
> that is the default in one of the many editors people use for editing
> the source code.

The default is not completely random. I believe the idea behind wrapping
lines a little earlier than 80 characters is that it gives you a buffer
for when the text is used in other contexts: our code often appears in
diffs on this list and these diffs are then quoted further in followup
replies.

Here, such a "buffer" can allow you to edit a line without having to
reflow the subsequent lines. So if you replace "foo" with "foobar" the
line lenght might still be within 80 characters.

> CodingStyle only mentions the 80 character limit. Reformatting to
> other conventions will just add unnecessary noise - especially when
> the next guy come by and want to wrap to 80 characters and has good
> arguments why that would be better.

In my experience, it's mostly people who use Emacs who even think about
reformatting text line that. My guess is that it's because it's so easy
and well-supported in Emacs. In my previous job, Sublime Text was quite
popular. When I asked people how it rewrapped text they looked at me
with puzzled look. Sublime Text actually supports reflowing of text, and
the keyboard shortcut is even the same as in Emacs: "Alt + q" or "M-q"
as Emacs people calls it. However, the results were pretty disasterous:
the editor also happily rewrapped the Python code :)

From a little bit of googling, I wasn't able to make Vim fare much
better. I tried with "gq}". That's probably just because I'm not a Vim
super user.
Mads Kiilerich - April 21, 2014, 2:51 p.m.
On 04/21/2014 02:07 PM, Martin Geisler wrote:

>>> I can resend with that fixed if you like? I would also like to wrap
>>> the strings at a consistent width (72 chars is default in Emacs), but
>>> I'm unsure if we want this on stable?
>> It do not seem to be a good idea to wrap it to 72 chars just because
>> that is the default in one of the many editors people use for editing
>> the source code.
> The default is not completely random. I believe the idea behind wrapping
> lines a little earlier than 80 characters is that it gives you a buffer
> for when the text is used in other contexts: our code often appears in
> diffs on this list and these diffs are then quoted further in followup
> replies.
>
> Here, such a "buffer" can allow you to edit a line without having to
> reflow the subsequent lines. So if you replace "foo" with "foobar" the
> line lenght might still be within 80 characters.

Sure. I am not saying that 72 characters is bad. But if we want to 
reformat code automatically, we should do it consistently, and describe 
and mandate it in CodingStyle.

> In my experience, it's mostly people who use Emacs who even think about
> reformatting text line that. My guess is that it's because it's so easy
> and well-supported in Emacs. In my previous job, Sublime Text was quite
> popular. When I asked people how it rewrapped text they looked at me
> with puzzled look. Sublime Text actually supports reflowing of text, and
> the keyboard shortcut is even the same as in Emacs: "Alt + q" or "M-q"
> as Emacs people calls it. However, the results were pretty disasterous:
> the editor also happily rewrapped the Python code :)

Should we make it mandatory to use emacs?

/Mads
Martin Geisler - April 21, 2014, 3:43 p.m.
Mads Kiilerich <mads@kiilerich.com> writes:

> On 04/21/2014 02:07 PM, Martin Geisler wrote:
>
>>>> I can resend with that fixed if you like? I would also like to wrap
>>>> the strings at a consistent width (72 chars is default in Emacs),
>>>> but I'm unsure if we want this on stable?
>>> It do not seem to be a good idea to wrap it to 72 chars just because
>>> that is the default in one of the many editors people use for
>>> editing the source code.
>> The default is not completely random. I believe the idea behind
>> wrapping lines a little earlier than 80 characters is that it gives
>> you a buffer for when the text is used in other contexts: our code
>> often appears in diffs on this list and these diffs are then quoted
>> further in followup replies.
>>
>> Here, such a "buffer" can allow you to edit a line without having to
>> reflow the subsequent lines. So if you replace "foo" with "foobar"
>> the line lenght might still be within 80 characters.
>
> Sure. I am not saying that 72 characters is bad. But if we want to
> reformat code automatically, we should do it consistently, and
> describe and mandate it in CodingStyle.

That feels like too much effort for my taste.

>> In my experience, it's mostly people who use Emacs who even think
>> about reformatting text line that. My guess is that it's because it's
>> so easy and well-supported in Emacs. In my previous job, Sublime Text
>> was quite popular. When I asked people how it rewrapped text they
>> looked at me with puzzled look. Sublime Text actually supports
>> reflowing of text, and the keyboard shortcut is even the same as in
>> Emacs: "Alt + q" or "M-q" as Emacs people calls it. However, the
>> results were pretty disasterous: the editor also happily rewrapped
>> the Python code :)
>
> Should we make it mandatory to use emacs?

No, no, I would never suggest that. I was just thinking that if it's
mostly people using Emacs who gets the idea of rewrapping the text and
if they use the Emacs default, well then there's no consistency problem.

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -6,7 +6,7 @@ 
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
-"""Obsolete markers handling
+"""Obsolete marker handling
 
 An obsolete marker maps an old changeset to a list of new
 changesets. If the list of new changesets is empty, the old changeset
@@ -14,30 +14,30 @@ 
 "replaced" by the new changesets.
 
 Obsolete markers can be used to record and distribute changeset graph
-transformations performed by history rewriting operations, and help
-building new tools to reconciliate conflicting rewriting actions. To
-facilitate conflicts resolution, markers include various annotations
+transformations performed by history rewrite operations, and help
+building new tools to reconcile conflicting rewrite actions. To
+facilitate conflict resolution, markers include various annotations
 besides old and news changeset identifiers, such as creation date or
 author name.
 
-The old obsoleted changeset is called "precursor" and possible replacements are
-called "successors".  Markers that used changeset X as a precursors are called
+The old obsoleted changeset is called a "precursor" and possible replacements are
+called "successors".  Markers that used changeset X as a precursor are called
 "successor markers of X" because they hold information about the successors of
 X. Markers that use changeset Y as a successors are call "precursor markers of
 Y" because they hold information about the precursors of Y.
 
 Examples:
 
-- When changeset A is replacement by a changeset A', one marker is stored:
+- When changeset A is replaced by changeset A', one marker is stored:
 
     (A, (A'))
 
-- When changesets A and B are folded into a new changeset C two markers are
+- When changesets A and B are folded into a new changeset C, two markers are
   stored:
 
     (A, (C,)) and (B, (C,))
 
-- When changeset A is simply "pruned" from the graph, a marker in create:
+- When changeset A is simply "pruned" from the graph, a marker is created:
 
     (A, ())
 
@@ -45,9 +45,9 @@ 
 
     (A, (C, C))
 
-  We use a single marker to distinct the "split" case from the "divergence"
-  case. If two independents operation rewrite the same changeset A in to A' and
-  A'' when have an error case: divergent rewriting. We can detect it because
+  We use a single marker to distinguish the "split" case from the "divergence"
+  case. If two independent operations rewrite the same changeset A in to A' and
+  A'', we have an error case: divergent rewriting. We can detect it because
   two markers will be created independently:
 
   (A, (B,)) and (A, (C,))
@@ -65,12 +65,12 @@ 
 
 The header is followed by the markers. Each marker is made of:
 
-- 1 unsigned byte: number of new changesets "R", could be zero.
+- 1 unsigned byte: number of new changesets "R", can be zero.
 
 - 1 unsigned 32-bits integer: metadata size "M" in bytes.
 
-- 1 byte: a bit field. It is reserved for flags used in obsolete
-  markers common operations, to avoid repeated decoding of metadata
+- 1 byte: a bit field. It is reserved for flags used in common
+  obsolete marker operations, to avoid repeated decoding of metadata
   entries.
 
 - 20 bytes: obsoleted changeset identifier.
@@ -78,7 +78,7 @@ 
 - N*20 bytes: new changesets identifiers.
 
 - M bytes: metadata as a sequence of nul-terminated strings. Each
-  string contains a key and a value, separated by a color ':', without
+  string contains a key and a value, separated by a colon ':', without
   additional encoding. Keys cannot contain '\0' or ':' and values
   cannot contain '\0'.
 """