bug#6411: Ispell string or comment

bug#6411: Ispell string or comment

[Lennart Borgman]

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

Here is a suggestion for a function ispell-comment-or-string for spell
checking of current comment or string, see attachement.

[-- Attachment #2: ispell-comment-0.diff --]
[-- Type: text/x-patch, Size: 1537 bytes --]

=== modified file 'lisp/textmodes/ispell.el'
--- trunk/lisp/textmodes/ispell.el	2010-05-13 10:36:03 +0000
+++ patched/lisp/textmodes/ispell.el	2010-06-12 17:53:38 +0000
@@ -3266,11 +3266,21 @@
 
 
 ;;;###autoload
-(defun ispell-comments-and-strings ()
+(defun ispell-comments-and-strings (only-current)
   "Check comments and strings in the current buffer for spelling errors."
-  (interactive)
+  (interactive (list nil))
+  (let (state done)
+    (if (not only-current)
   (goto-char (point-min))
-  (let (state done)
+      (let* ((here (point))
+             (state (parse-partial-sexp (point-min) (point)))
+             (inside (or (nth 3 state) (nth 4 state)))
+             (start (when inside (nth 8 state))))
+        (if start
+            (goto-char start)
+          (setq done t)
+          (goto-char here)
+          (message "Not inside a string or comment"))))
     (while (not done)
       (setq done t)
       (setq state (parse-partial-sexp (point) (point-max)
@@ -3282,8 +3292,14 @@
 	    (if (or (nth 3 state) (nth 4 state))
 		(error "Unterminated string or comment"))
 	    (save-excursion
-	      (setq done (not (ispell-region start (point))))))))))
+	      (setq done (not (ispell-region start (point)))))))
+      (when only-current (setq done t)))))
 
+;;;###autoload
+(defun ispell-comment-or-string ()
+  "Check comments or strings at point for spelling errors."
+  (interactive)
+  (ispell-comments-and-strings t))
 
 ;;;###autoload
 (defun ispell-buffer ()

bug#6411: Ispell string or comment

[Agustin Martin]

On Sat, Jun 12, 2010 at 07:57:43PM +0200, Lennart Borgman wrote:
> Here is a suggestion for a function ispell-comment-or-string for spell
> checking of current comment or string, see attachement.

Seems OK to me, specially for people using keyboard more than mouse.

-- 
Agustin

bug#6411: Ispell string or comment

[Stefan Kangas]

tags 6411 + patch
quit

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Here is a suggestion for a function ispell-comment-or-string for spell
> checking of current comment or string, see attachement.

This patch from 2010 still applies on current master, and it seems to
work as advertised from my minimal testing.  I think it would be a
good addition.  Does anyone object to installing it?

Thanks,
Stefan Kangas

bug#6411: Ispell string or comment

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 21 Aug 2019 16:07:53 +0200
> Cc: 6411@debbugs.gnu.org
> 
> Lennart Borgman <lennart.borgman@gmail.com> writes:
> 
> > Here is a suggestion for a function ispell-comment-or-string for spell
> > checking of current comment or string, see attachement.
> 
> This patch from 2010 still applies on current master, and it seems to
> work as advertised from my minimal testing.  I think it would be a
> good addition.  Does anyone object to installing it?

Sorry, but I happen to think the proposed implementation is somewhat
inelegant:

  . what's with the "(interactive (list nil))" part?
  . why not just make ispell-comments-and-strings accept a prefix
    argument to modify its behavior?
  . alternatively, why not make ispell-comments-and-strings be
    sensitive to the active region?
  . as yet another alternative, make the new ispell-comment-or-string
    narrow to the comment/string at point and then call
    ispell-comments-and-strings with no code changes whatsoever; then
    make the new command be sensitive to active region, and fall back
    on the string/comment at point if no active region

bug#6411: Ispell string or comment

[Stefan Kangas]

tags 6411 - patch
quit

Eli Zaretskii <eliz@gnu.org> writes:

> > > Here is a suggestion for a function ispell-comment-or-string for spell
> > > checking of current comment or string, see attachement.
> >
> > This patch from 2010 still applies on current master, and it seems to
> > work as advertised from my minimal testing.  I think it would be a
> > good addition.  Does anyone object to installing it?
>
> Sorry, but I happen to think the proposed implementation is somewhat
> inelegant:

Thanks, many good points that I agree with.  I will review the
proposed patch more carefully next time beyond just testing that it
works.

Let's hope someone picks up the ball and provides us with a revised or
new patch.  If Lennart Borgman doesn't feel up to it after all these
years, maybe someone else?

Best regards,
Stefan Kangas

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Štěpán Němec]

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

tags 6411 + patch
thanks

On Wed, 21 Aug 2019 20:08:29 +0300
Eli Zaretskii wrote:

> Sorry, but I happen to think the proposed implementation is somewhat
> inelegant:
>
>   . what's with the "(interactive (list nil))" part?
>   . why not just make ispell-comments-and-strings accept a prefix
>     argument to modify its behavior?
>   . alternatively, why not make ispell-comments-and-strings be
>     sensitive to the active region?
>   . as yet another alternative, make the new ispell-comment-or-string
>     narrow to the comment/string at point and then call
>     ispell-comments-and-strings with no code changes whatsoever; then
>     make the new command be sensitive to active region, and fall back
>     on the string/comment at point if no active region

On Wed, 21 Aug 2019 22:57:21 +0200
Stefan Kangas wrote:

> Let's hope someone picks up the ball and provides us with a revised or
> new patch.  If Lennart Borgman doesn't feel up to it after all these
> years, maybe someone else?

Here's my take on it.

I didn't go the narrowing route, as I think it is useful to have full
context during spelling corrections (recursive edit etc.).

I also didn't see an obvious way to reuse one function/command from the
other, as in order to get bounds of a comment/string, we have to parse
the (part of the) buffer, by which point we can just pass the bounds
straight to `ispell-region'.

I did update `ispell-comments-and-strings' to honour active region and
take optional bounds, anyway, as it seems useful in itself.

One other possibility I see would be to merge the two commands and
condition the "at-point" functionality on a prefix argument, but then
the name ('ispell-comments-and-strings') would probably have to be
changed, anyway (and to what?), and perhaps the command would already
become too much of a kitchen sink.

-- 
Štěpán


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ispell-Commands-to-check-comments-or-strings-at-poin.patch --]
[-- Type: text/x-patch, Size: 3221 bytes --]

From 9ea826c09e848f18b9c09e23c00d525da26d280b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Wed, 8 Apr 2020 20:32:51 +0200
Subject: [PATCH] ispell: Commands to check comments or strings at point or in
 region

* lisp/textmodes/ispell.el (ispell-comments-and-strings): Accept START
and END arguments, defaulting to active region in interactive calls.
(ispell-comment-or-string-at-point): New command. (bug#6411)
---
 etc/NEWS                 |  7 +++++++
 lisp/textmodes/ispell.el | 30 +++++++++++++++++++++++-------
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index fa5478679f..f7f18d12b9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -144,6 +144,13 @@ this user option.
 +++
 *** New command 'describe-keymap' describes keybindings in a keymap.
 
+** Ispell
+---
+*** 'ispell-comments-and-strings' now accepts START and END arguments,
+defaulting to active region when used interactively.
+---
+*** New command 'ispell-comment-or-string-at-point' is provided.
+
 ---
 ** The old non-SMIE indentation of 'sh-mode' has been removed.
 
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index a9fbd2f04c..e1642a8d87 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -3591,24 +3591,40 @@ ispell-process-line
 
 
 ;;;###autoload
-(defun ispell-comments-and-strings ()
-  "Check comments and strings in the current buffer for spelling errors."
-  (interactive)
-  (goto-char (point-min))
+(defun ispell-comments-and-strings (&optional start end)
+  "Check comments and strings in the current buffer for spelling errors.
+If called interactively with an active region, check only comments and
+strings in the region.
+When called from Lisp, START and END buffer positions can be provided
+to limit the check."
+  (interactive (when (use-region-p) (list (region-beginning) (region-end))))
+  (unless end (setq end (point-max)))
+  (goto-char (or start (point-min)))
   (let (state done)
     (while (not done)
       (setq done t)
-      (setq state (parse-partial-sexp (point) (point-max)
-				      nil nil state 'syntax-table))
+      (setq state (parse-partial-sexp (point) end nil nil state 'syntax-table))
       (if (or (nth 3 state) (nth 4 state))
 	  (let ((start (point)))
-	    (setq state (parse-partial-sexp start (point-max)
+	    (setq state (parse-partial-sexp start end
 					    nil nil state 'syntax-table))
 	    (if (or (nth 3 state) (nth 4 state))
 		(error "Unterminated string or comment"))
 	    (save-excursion
 	      (setq done (not (ispell-region start (point))))))))))
 
+;;;###autoload
+(defun ispell-comment-or-string-at-point ()
+  "Check the comment or string containing point for spelling errors."
+  (interactive)
+  (save-excursion
+    (let ((state (syntax-ppss)))
+      (if (or (nth 3 state) (nth 4 state))
+          (ispell-region (nth 8 state)
+                         (progn (parse-partial-sexp (point) (point-max)
+                                                    nil nil state 'syntax-table)
+                                (point)))
+        (user-error "Not inside a string or comment")))))
 
 ;;;###autoload
 (defun ispell-buffer ()
-- 
2.26.0

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: Stefan Kangas <stefan@marxist.se>,  lennart.borgman@gmail.com,
>   6411@debbugs.gnu.org
> Date: Wed, 08 Apr 2020 21:23:20 +0200
> 
> > Let's hope someone picks up the ball and provides us with a revised or
> > new patch.  If Lennart Borgman doesn't feel up to it after all these
> > years, maybe someone else?
> 
> Here's my take on it.

LGTM, thanks.

Let's wait for a few days for comments, before pushing this.

I also wonder whether it's time to document
ispell-comments-and-strings in the user manual.  Any opinions?

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Štěpán Němec]

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

On Thu, 09 Apr 2020 09:21:34 +0300
Eli Zaretskii wrote:

> I also wonder whether it's time to document
> ispell-comments-and-strings in the user manual.  Any opinions?

I considered it, but from a skim through (info "(emacs) Spelling") it
seemed that some other commands were missing, too, and the new commands
should be easily discoverable anyway.

But on second look I see that the only one really missing is
`ispell-buffer-with-debug', which probably shouldn't be documented, and
`ispell-continue', which _is_ actually documented in the UI explanation,
only not by its name (as it is normally invoked as C-u M-$).

So I now think it would be better to mention the comment/string
commands, too, perhaps as follows:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Document-ispell-comment-string-checking-commands-in-.patch --]
[-- Type: text/x-patch, Size: 2108 bytes --]

From 5b1abf142d090d35d1b62ce45b1716c47fc0c667 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Thu, 9 Apr 2020 16:17:57 +0200
Subject: [PATCH] Document ispell comment/string checking commands in the user
 manual

* doc/emacs/fixit.texi (Spelling): Mention
ispell-comments-and-strings and ispell-comment-or-string-at-point.
---
 doc/emacs/fixit.texi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/fixit.texi b/doc/emacs/fixit.texi
index 2c268f14fb..c6a14c99c0 100644
--- a/doc/emacs/fixit.texi
+++ b/doc/emacs/fixit.texi
@@ -277,6 +277,10 @@ Spelling
 @item M-x ispell-message
 Check and correct spelling in a draft mail message, excluding cited
 material.
+@item M-x ispell-comments-and-strings
+Check and correct spelling of comments and strings in the buffer or region.
+@item M-x ispell-comment-or-string-at-point
+Check the comment or string at point.
 @item M-x ispell-change-dictionary @key{RET} @var{dict} @key{RET}
 Restart the spell-checker process, using @var{dict} as the dictionary.
 @item M-x ispell-kill-ispell
@@ -304,6 +308,8 @@ Spelling
 @findex ispell
 @findex ispell-buffer
 @findex ispell-region
+@findex ispell-comments-and-strings
+@findex ispell-comment-or-string-at-point
 @cindex spell-checking the active region
   Similarly, the command @kbd{M-x ispell} performs spell-checking in
 the region if one is active, or in the entire buffer otherwise.  The
@@ -312,7 +318,10 @@ Spelling
 respectively.  To check spelling in an email message you are writing,
 use @kbd{M-x ispell-message}; that command checks the whole buffer,
 except for material that is indented or appears to be cited from other
-messages.  @xref{Sending Mail}.
+messages.  @xref{Sending Mail}.  When dealing with source code, you
+can use @kbd{M-x ispell-comments-and-strings} or @kbd{M-x
+ispell-comment-or-string-at-point} to check only comments or string
+literals.
 
   When one of these commands encounters what appears to be an
 incorrect word, it asks you what to do.  It usually displays a list of
-- 
2.26.0

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: stefan@marxist.se,  lennart.borgman@gmail.com,  6411@debbugs.gnu.org
> Date: Thu, 09 Apr 2020 16:48:30 +0200
> 
> So I now think it would be better to mention the comment/string
> commands, too, perhaps as follows:

Thanks.  A few minor nits below:

> * doc/emacs/fixit.texi (Spelling): Mention
> ispell-comments-and-strings and ispell-comment-or-string-at-point.

Please quote symbols in log messages 'like this'.  Please also mention
the bug number there.

> -messages.  @xref{Sending Mail}.
> +messages.  @xref{Sending Mail}.  When dealing with source code, you
> +can use @kbd{M-x ispell-comments-and-strings} or @kbd{M-x
> +ispell-comment-or-string-at-point} to check only comments or string

When a long "M-x SOMETHING" command is close to a line's end, please
enclose it in @w{..}, so that it doesn't get split between two lines.

Other than these two minor issues, this LGTM, thanks.

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Eli Zaretskii]

> Date: Thu, 09 Apr 2020 20:06:04 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: stefan@marxist.se, lennart.borgman@gmail.com, 6411@debbugs.gnu.org
> 
> > From: Štěpán Němec <stepnem@gmail.com>
> > Cc: stefan@marxist.se,  lennart.borgman@gmail.com,  6411@debbugs.gnu.org
> > Date: Thu, 09 Apr 2020 16:48:30 +0200
> > 
> > So I now think it would be better to mention the comment/string
> > commands, too, perhaps as follows:
> 
> Thanks.  A few minor nits below:

And one other thing: since these changes are now described in the
manual, the NEWS entry should be marked with "+++".

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Štěpán Němec]

On Thu, 09 Apr 2020 20:06:04 +0300
Eli Zaretskii wrote:

>> From: Štěpán Němec <stepnem@gmail.com>
>> Cc: stefan@marxist.se,  lennart.borgman@gmail.com,  6411@debbugs.gnu.org
>> Date: Thu, 09 Apr 2020 16:48:30 +0200
>> 
>> So I now think it would be better to mention the comment/string
>> commands, too, perhaps as follows:
>
> Thanks.  A few minor nits below:
>
>> * doc/emacs/fixit.texi (Spelling): Mention
>> ispell-comments-and-strings and ispell-comment-or-string-at-point.
>
> Please quote symbols in log messages 'like this'.  Please also mention
> the bug number there.

Will fix, thank you. (Funnily enough I did double check this when making
the patch, and found recent commit messages quite diverse in that
regard.)

>> -messages.  @xref{Sending Mail}.
>> +messages.  @xref{Sending Mail}.  When dealing with source code, you
>> +can use @kbd{M-x ispell-comments-and-strings} or @kbd{M-x
>> +ispell-comment-or-string-at-point} to check only comments or string
>
> When a long "M-x SOMETHING" command is close to a line's end, please
> enclose it in @w{..}, so that it doesn't get split between two lines.

By "close to a line's end" I assume you mean to check the rendered info
file, as IIUC there is no strict correspondence between the texi source
and the final document WRT end of line?

-- 
Štěpán

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: stefan@marxist.se,  lennart.borgman@gmail.com,  6411@debbugs.gnu.org
> Date: Thu, 09 Apr 2020 20:44:59 +0200
> 
> > Please quote symbols in log messages 'like this'.  Please also mention
> > the bug number there.
> 
> Will fix, thank you. (Funnily enough I did double check this when making
> the patch, and found recent commit messages quite diverse in that
> regard.)

Yes, not everybody follows the policy (and I'm ashamed to sometimes
forget doing that as well...)

> >> +can use @kbd{M-x ispell-comments-and-strings} or @kbd{M-x
> >> +ispell-comment-or-string-at-point} to check only comments or string
> >
> > When a long "M-x SOMETHING" command is close to a line's end, please
> > enclose it in @w{..}, so that it doesn't get split between two lines.
> 
> By "close to a line's end" I assume you mean to check the rendered info
> file, as IIUC there is no strict correspondence between the texi source
> and the final document WRT end of line?

The correspondence is quite close, even though not 100%, and there's
the Info output, where it's even closer.  So you could just use
@w{@kbd{..}} when it's close in Texinfo, because @w{..} is harmless if
it isn't needed.

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Štěpán Němec]

On Thu, 09 Apr 2020 22:01:58 +0300
Eli Zaretskii wrote:

>> > When a long "M-x SOMETHING" command is close to a line's end, please
>> > enclose it in @w{..}, so that it doesn't get split between two lines.
>> 
>> By "close to a line's end" I assume you mean to check the rendered info
>> file, as IIUC there is no strict correspondence between the texi source
>> and the final document WRT end of line?
>
> The correspondence is quite close, even though not 100%, and there's
> the Info output, where it's even closer.  So you could just use
> @w{@kbd{..}} when it's close in Texinfo, because @w{..} is harmless if
> it isn't needed.

But the intent is not to have "M-x" on one line and "some-command" on
the next in the rendered output, right? Because that's exactly what
happens e.g. in (info "(emacs) Spelling") with "M-x ispell-message" ("To
check spelling in an email message you are writing, use ‘M-x
ispell-message’"), even though in fixit.texi "M-x ispell-message" is
nowhere near end of line, so it still seems to me that "close to a
line's end in Texinfo" as a rule isn't all that helpful. I guess one
had better always check the Info output, or just use @w{} for M-x
commands always, unless clearly unnecessary (e.g. a sole line).

-- 
Štěpán

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: stefan@marxist.se,  lennart.borgman@gmail.com,  6411@debbugs.gnu.org
> Date: Thu, 09 Apr 2020 21:56:51 +0200
> 
> But the intent is not to have "M-x" on one line and "some-command" on
> the next in the rendered output, right?

Yes.

> I guess one had better always check the Info output, or just use
> @w{} for M-x commands always

Yes.  It's a good idea to do that anyway, because many times you will
see in the Info output something you missed in Texinfo.

bug#6411: Ispell string or comment, bug#6411: Ispell string or comment

[Štěpán Němec]

On Thu, 09 Apr 2020 09:21:34 +0300
Eli Zaretskii wrote:

>> Here's my take on it.
>
> LGTM, thanks.
>
> Let's wait for a few days for comments, before pushing this.

Many a week has passed without further comments, so I have now pushed
the changes to master, having addressed the points raised:

2020-04-08T20:32:51+02:00!stepnem@gmail.com
2aefd55904 (ispell: Commands to check comments or strings at point or in region)
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=2aefd55904

2020-04-09T16:17:57+02:00!stepnem@gmail.com
e48c60e7c9 (Document ispell comment/string checking commands in the user manual)
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=e48c60e7c9

I am closing this bug report.

Thank you and sorry for the delay,

  Štěpán