bug#32651: flyspell marks doublons when not the same case

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

Flyspell marks for example the second "AND" in "an AND split and AND join"
as a duplicate.

This seems to be deliberate: in line 1153 of flyspell.el (on current
emacs-26 branch) flyspell-word-search-backward is called with IGNORE-CASE
set to t.

I don't understand this: if it's not the same case, it's probably
deliberate, no?

I checked that simply removing this `t' argument makes doublon detection
work as I'd expect: "AND AND", "and and" and "And And" are all marked as
doublons, but anything that's not the same case isn't.

I'm using Emacs 26.1.

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1806 bytes --]

bug#32651: flyspell marks doublons when not the same case

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Thu, 6 Sep 2018 20:16:10 +0100
> 
> Flyspell marks for example the second "AND" in "an AND split and AND join" as a duplicate.
> 
> This seems to be deliberate: in line 1153 of flyspell.el (on current emacs-26 branch)
> flyspell-word-search-backward is called with IGNORE-CASE set to t.
> 
> I don't understand this: if it's not the same case, it's probably deliberate, no?
> 
> I checked that simply removing this `t' argument makes doublon detection work as I'd expect: "AND AND",
> "and and" and "And And" are all marked as doublons, but anything that's not the same case isn't.

There are valid use cases where the current behavior is what the users
want (e.g., "And and" at the beginning of a sentence).  So we could
introduce an option to do what you want, but I don't think we can
change the default behavior unconditionally.

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

On 6 September 2018 at 20:34, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Thu, 6 Sep 2018 20:16:10 +0100
> >
> > Flyspell marks for example the second "AND" in "an AND split and AND
> join" as a duplicate.
> >
> > This seems to be deliberate: in line 1153 of flyspell.el (on current
> emacs-26 branch)
> > flyspell-word-search-backward is called with IGNORE-CASE set to t.
> >
> > I don't understand this: if it's not the same case, it's probably
> deliberate, no?
> >
> > I checked that simply removing this `t' argument makes doublon detection
> work as I'd expect: "AND AND",
> > "and and" and "And And" are all marked as doublons, but anything that's
> not the same case isn't.
>
> There are valid use cases where the current behavior is what the users
> want (e.g., "And and" at the beginning of a sentence).  So we could
> introduce an option to do what you want, but I don't think we can
> change the default behavior unconditionally.
>

I am suggesting the above example is not a valid use case, because it's
unlikely that someone would type that by mistake; rather, it's typing the
same identical word twice that is the common error.

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1962 bytes --]

bug#32651: flyspell marks doublons when not the same case

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Thu, 6 Sep 2018 20:37:04 +0100
> Cc: 32651@debbugs.gnu.org
> 
>  There are valid use cases where the current behavior is what the users
>  want (e.g., "And and" at the beginning of a sentence).  So we could
>  introduce an option to do what you want, but I don't think we can
>  change the default behavior unconditionally.
> 
> I am suggesting the above example is not a valid use case, because it's unlikely that someone would type
> that by mistake; rather, it's typing the same identical word twice that is the common error.

I don't understand how you conclude that these are not valid use
cases.  E.g., I could have a mode turned on that automatically
capitalizes words at the beginning of each sentence, in which case
typing "and "and" will produce "And and".  And that is just an example
based on 5 sec of thought.

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On 6 September 2018 at 20:53, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Thu, 6 Sep 2018 20:37:04 +0100
> > Cc: 32651@debbugs.gnu.org
> >
> >  There are valid use cases where the current behavior is what the users
> >  want (e.g., "And and" at the beginning of a sentence).  So we could
> >  introduce an option to do what you want, but I don't think we can
> >  change the default behavior unconditionally.
> >
> > I am suggesting the above example is not a valid use case, because it's
> unlikely that someone would type
> > that by mistake; rather, it's typing the same identical word twice that
> is the common error.
>
> I don't understand how you conclude that these are not valid use
> cases.  E.g., I could have a mode turned on that automatically
> capitalizes words at the beginning of each sentence, in which case
> typing "and "and" will produce "And and".  And that is just an example
> based on 5 sec of thought.
>

And I don't understand how, just because you can think of an example, that
makes the current behaviour a good default! I think we'll have to disagree.
I would be happy with a preference, of course.

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1979 bytes --]

bug#32651: flyspell marks doublons when not the same case

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Thu, 6 Sep 2018 20:58:48 +0100
> Cc: 32651@debbugs.gnu.org
> 
>  I don't understand how you conclude that these are not valid use
>  cases.  E.g., I could have a mode turned on that automatically
>  capitalizes words at the beginning of each sentence, in which case
>  typing "and "and" will produce "And and".  And that is just an example
>  based on 5 sec of thought.
> 
> And I don't understand how, just because you can think of an example, that makes the current behaviour a
> good default! I think we'll have to disagree. I would be happy with a preference, of course.

I'm not claiming the current behavior is a good default, I'm saying it
is there for a long time, and thus takes precedence, because we cannot
possibly poll all those users who see that behavior every day and none
of them complained.

This is why I suggested a new customizable option: that way, you and
anyone else who wants Flyspell to behave differently can have that,
and if we learn this is the preferred behavior (by getting requests to
set that as default), we can make it the default at some future point.
IOW, what I suggested is a path towards potentially changing the
default, but gradually and without backward incompatibilities.  OK?

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On 7 September 2018 at 06:59, Eli Zaretskii <eliz@gnu.org> wrote:

>
> I'm not claiming the current behavior is a good default, I'm saying it
> is there for a long time, and thus takes precedence, because we cannot
> possibly poll all those users who see that behavior every day and none
> of them complained.
>
> This is why I suggested a new customizable option: that way, you and
> anyone else who wants Flyspell to behave differently can have that,
> and if we learn this is the preferred behavior (by getting requests to
> set that as default), we can make it the default at some future point.
> IOW, what I suggested is a path towards potentially changing the
> default, but gradually and without backward incompatibilities.  OK?
>

OK!

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1378 bytes --]

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 937 bytes --]

On 7 September 2018 at 10:49, Reuben Thomas <rrt@sc3d.org> wrote:

> On 7 September 2018 at 06:59, Eli Zaretskii <eliz@gnu.org> wrote:
>
>>
>> I'm not claiming the current behavior is a good default, I'm saying it
>> is there for a long time, and thus takes precedence, because we cannot
>> possibly poll all those users who see that behavior every day and none
>> of them complained.
>>
>> This is why I suggested a new customizable option: that way, you and
>> anyone else who wants Flyspell to behave differently can have that,
>> and if we learn this is the preferred behavior (by getting requests to
>> set that as default), we can make it the default at some future point.
>> IOW, what I suggested is a path towards potentially changing the
>> default, but gradually and without backward incompatibilities.  OK?
>>
>
> OK!
>
Attached, a patch. If the code's OK, I can write a NEWS entry (anything
else?).

-- 
https://rrt.sc3d.org

[-- Attachment #1.2: Type: text/html, Size: 2141 bytes --]

[-- Attachment #2: flyspell-case-fold-doublons-option.patch --]
[-- Type: text/x-patch, Size: 1054 bytes --]

diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index 4d7a18969e..cf209abf6a 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -68,6 +68,11 @@ flyspell-mark-duplications-flag
   :group 'flyspell
   :type 'boolean)
 
+(defcustom flyspell-case-fold-duplications t
+  "Non-nil means Flyspell matches duplicate words case-insensitively."
+  :group 'flyspell
+  :type 'boolean)
+
 (defcustom flyspell-mark-duplications-exceptions
   '((nil . ("that" "had")) ; Common defaults for English.
     ("\\`francais" . ("nous" "vous")))
@@ -1150,7 +1155,8 @@ flyspell-word
 			      (- (save-excursion
                                    (skip-chars-backward " \t\n\f")))))
 			  (p (when (>= bound (point-min))
-			       (flyspell-word-search-backward word bound t))))
+			       (flyspell-word-search-backward
+                                word bound flyspell-case-fold-duplications))))
 		     (and p (/= p start)))))
 	    ;; yes, this is a doublon
 	    (flyspell-highlight-incorrect-region start end 'doublon)

bug#32651: flyspell marks doublons when not the same case

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Sun, 9 Sep 2018 20:45:51 +0100
> Cc: 32651@debbugs.gnu.org
> 
>  This is why I suggested a new customizable option: that way, you and
>  anyone else who wants Flyspell to behave differently can have that,
>  and if we learn this is the preferred behavior (by getting requests to
>  set that as default), we can make it the default at some future point.
>  IOW, what I suggested is a path towards potentially changing the
>  default, but gradually and without backward incompatibilities.  OK?
> 
>  OK!
> 
> Attached, a patch. If the code's OK, I can write a NEWS entry (anything else?).

NEWS should be enough, I think.  Also...

> +(defcustom flyspell-case-fold-duplications t
> +  "Non-nil means Flyspell matches duplicate words case-insensitively."
> +  :group 'flyspell
> +  :type 'boolean)

This defcustom should have the :version tag.

Thanks.

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 460 bytes --]

On 9 September 2018 at 20:54, Eli Zaretskii <eliz@gnu.org> wrote:

> NEWS should be enough, I think.  Also...
>
> > +(defcustom flyspell-case-fold-duplications t
> > +  "Non-nil means Flyspell matches duplicate words case-insensitively."
> > +  :group 'flyspell
> > +  :type 'boolean)
>
> This defcustom should have the :version tag.
>

Thanks. How's the attached? If it's OK, I can install it (with suitable
ChangeLog entry) myself.

-- 
https://rrt.sc3d.org

[-- Attachment #1.2: Type: text/html, Size: 1164 bytes --]

[-- Attachment #2: flyspell-case-fold-doublons-option.patch --]
[-- Type: text/x-patch, Size: 1771 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index ff65a5520d..587e17d3f3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -231,6 +231,13 @@ characters that quote text "like this" are replaced by double
 typographic quotes, “like this”, in text modes, and in comments in
 non-text modes.
 
++++
+** New user option 'flyspell-case-fold-duplications'
+This option controls whether Flyspell mode considers consecutive words
+to be duplicates if they are not in the same case.  If non-nil, the
+default, words are considered to be duplicates even if their letters'
+case does not match.
+
 ---
 ** 'write-abbrev-file' now includes special properties.
 'write-abbrev-file' now writes special properties like ':case-fixed'
diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index f6a809b18e..e5a7639e20 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -68,6 +68,12 @@ flyspell-mark-duplications-flag
   :group 'flyspell
   :type 'boolean)
 
+(defcustom flyspell-case-fold-duplications t
+  "Non-nil means Flyspell matches duplicate words case-insensitively."
+  :group 'flyspell
+  :type 'boolean
+  :version 27.1)
+
 (defcustom flyspell-mark-duplications-exceptions
   '((nil . ("that" "had")) ; Common defaults for English.
     ("\\`francais" . ("nous" "vous")))
@@ -1154,7 +1160,8 @@ flyspell-word
 			      (- (save-excursion
                                    (skip-chars-backward " \t\n\f")))))
 			  (p (when (>= bound (point-min))
-			       (flyspell-word-search-backward word bound t))))
+			       (flyspell-word-search-backward
+                                word bound flyspell-case-fold-duplications))))
 		     (and p (/= p start)))))
 	    ;; yes, this is a doublon
 	    (flyspell-highlight-incorrect-region start end 'doublon)

bug#32651: flyspell marks doublons when not the same case

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Sun, 9 Sep 2018 21:24:15 +0100
> Cc: 32651@debbugs.gnu.org
> 
> Thanks. How's the attached? If it's OK, I can install it (with suitable ChangeLog entry) myself.

OK, with two nits:

> ++++

This should be "---", since we aren't going to document this variable
in the manual.

> +** New user option 'flyspell-case-fold-duplications'

This should end with a period, to be a complete sentence.

Thanks.

bug#32651: flyspell marks doublons when not the same case

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On 10 September 2018 at 07:26, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Sun, 9 Sep 2018 21:24:15 +0100
> > Cc: 32651@debbugs.gnu.org
> >
> > Thanks. How's the attached? If it's OK, I can install it (with suitable
> ChangeLog entry) myself.
>
> OK, with two nits:
>
> > ++++
>
> This should be "---", since we aren't going to document this variable
> in the manual.
>
> > +** New user option 'flyspell-case-fold-duplications'
>
> This should end with a period, to be a complete sentence.
>
> Thanks.
>

Installed as commit 61f3a4b4f.

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1361 bytes --]

bug#32651: Installed as commit 61f3a4b4f

[Reuben Thomas]

[-- Attachment #1: Type: text/plain, Size: 25 bytes --]

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 308 bytes --]