bug#64104: Subject: 30.0.50; uncomment-region-default-1

bug#64104: Subject: 30.0.50; uncomment-region-default-1

[Andreas Röhler]

Hi,

C-u comment-dwim RET with an active region of the following Python code:

---
# for i in text:
#     print("(i): {}".format(str(i)))
---

results in error saying

uncomment-region-default-1: Beginning of buffer

AFAIU the culprit is in newcomment.el, line 933:

(numarg (prefix-numeric-value arg))

and later in line 978

it tries

(delete-char (- numarg))

which at the beginning of buffer causes that error.

comment-dwin says:

if a prefix ARG is specified, call ‘comment-kill’

which is my intended use-case.

Thanks,

Andreas Röhler

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
  3.24.33, cairo version 1.16.0) of 2023-06-16 built on kanu
Repository revision: f2aae8b879baa4986b69af7ee54fe4eb987b99d6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.2 LTS

Configured using:
  'configure --with-tree-sitter --with-modules --with-mailutils'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG
RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE
XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: de_DE.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Python

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny rfc822
mml mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils vc-git diff-mode easy-mmode vc-dispatcher
python json map rx project byte-opt gv bytecomp byte-compile pcase
treesit cl-seq comint ansi-osc ring ansi-color dired-aux dired
dired-loaddefs time-date subr-x wheatgrass-theme cus-theme cus-edit pp
cus-start cus-load icons wid-edit cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 135452 11240)
  (symbols 48 20124 0)
  (strings 32 48692 1931)
  (string-bytes 1 1037720)
  (vectors 16 17224)
  (vector-slots 8 230171 13481)
  (floats 8 39 46)
  (intervals 56 1109 13)
  (buffers 984 15))

bug#64104: Subject: 30.0.50; uncomment-region-default-1

[Eli Zaretskii]

> Date: Fri, 16 Jun 2023 10:51:24 +0200
> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
> 
> C-u comment-dwim RET with an active region of the following Python code:
> 
> ---
> # for i in text:
> #     print("(i): {}".format(str(i)))
> ---
> 
> results in error saying
> 
> uncomment-region-default-1: Beginning of buffer

AFAICT, that's because invoking comment-dwim with C-u in this case
makes no sense: the comment delimiters have just 1 character in this
case.  Try "C-u 1 M-x comment-dwim" instead.

> comment-dwin says:
> 
> if a prefix ARG is specified, call ‘comment-kill’
> 
> which is my intended use-case.

If this is what you wanted, then you should invoke "C-u comment-dwin"
without having an active region.  The doc string says that if the
region is active, it will call comment-region or uncomment-region, and
will call comment-kill only "otherwise" (i.e. if there's no active
region).  I suggest to read the node "Comment Commands" in the Emacs
manual, it describes this in more detail.

I've now made the doc string slightly more detailed, especially wrt
how the prefix argument is used in the various use cases, but other
than that, I don't think there's a bug here.

Stefan, any comments?

bug#64104: Subject: 30.0.50; uncomment-region-default-1

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> C-u comment-dwim RET with an active region of the following Python code:
>> 
>> ---
>> # for i in text:
>> #     print("(i): {}".format(str(i)))
>> ---
>> 
>> results in error saying
>> 
>> uncomment-region-default-1: Beginning of buffer

> Stefan, any comments?

- The error we give is not very user-friendly, so it might be good to
  catch it and re-raise it as something more meaning ful the specific
  operation performed.

- I have the vague impression that in some earlier version of that code we
  would not signal an error but just remove as many comment markers as
  possible (but no more than requested).
  That could be a more friendly behavior.


        Stefan

bug#64104: Subject: 30.0.50; uncomment-region-default-1

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Andreas Röhler <andreas.roehler@easy-emacs.de>,
>   64104@debbugs.gnu.org
> Date: Sat, 17 Jun 2023 14:39:08 -0400
> 
> >> C-u comment-dwim RET with an active region of the following Python code:
> >> 
> >> ---
> >> # for i in text:
> >> #     print("(i): {}".format(str(i)))
> >> ---
> >> 
> >> results in error saying
> >> 
> >> uncomment-region-default-1: Beginning of buffer
> 
> > Stefan, any comments?
> 
> - The error we give is not very user-friendly, so it might be good to
>   catch it and re-raise it as something more meaning ful the specific
>   operation performed.
> 
> - I have the vague impression that in some earlier version of that code we
>   would not signal an error but just remove as many comment markers as
>   possible (but no more than requested).
>   That could be a more friendly behavior.

I'll leave the bug open, then.  Patches to improve this command are
welcome.

bug#64104: Subject: 30.0.50; uncomment-region-default-1

[Andreas Röhler]

Am 18.06.23 um 07:53 schrieb Eli Zaretskii:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Andreas Röhler <andreas.roehler@easy-emacs.de>,
>>    64104@debbugs.gnu.org
>> Date: Sat, 17 Jun 2023 14:39:08 -0400
>>
>>>> C-u comment-dwim RET with an active region of the following Python code:
>>>>
>>>> ---
>>>> # for i in text:
>>>> #     print("(i): {}".format(str(i)))
>>>> ---
>>>>
>>>> results in error saying
>>>>
>>>> uncomment-region-default-1: Beginning of buffer
>>> Stefan, any comments?
>> - The error we give is not very user-friendly, so it might be good to
>>    catch it and re-raise it as something more meaning ful the specific
>>    operation performed.
>>
>> - I have the vague impression that in some earlier version of that code we
>>    would not signal an error but just remove as many comment markers as
>>    possible (but no more than requested).
>>    That could be a more friendly behavior.
> I'll leave the bug open, then.  Patches to improve this command are
> welcome.


Unfortunately don't have a trivial fix.

Suggest a simplified caller:

(defun comment-dwim (arg)
   "Call the comment command you want (Do What I Mean).

Comment or uncomment the current line, or, if active, the region.

With \\[universal-argument] copy line or region before changing it.

You can configure `comment-style' to change the way regions are commented."
   (interactive "*P")
   (comment-normalize-vars)
   (when (eq 4 (prefix-numeric-value))
     (copy-region-as-kill (or (region-beginning) 
(line-beginning-position)) (or (region-end) (line-end-position))))
   (comment-or-uncomment-region (or (region-beginning) 
(line-beginning-position)) (or (region-end)) (line-end-position)))