bug#59350: 29.0.50; Eval region or buffer

bug#59350: 29.0.50; Eval region or buffer

[Juri Linkov]

'elisp-eval-buffer' is a nice new command that evaluates the current buffer
with a convenient keybinding 'C-c C-e'.  It would be nicer also to react on
the currently active region:

```
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 6bdaa7a37a..9b8a66c243 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -2213,10 +2213,14 @@ elisp-flymake--batch-compile-for-flymake
     (pp collected)))
 
 (defun elisp-eval-buffer ()
-  "Evaluate the forms in the current buffer."
+  "Evaluate the forms in the active region or the whole current buffer."
   (interactive)
-  (eval-buffer)
-  (message "Evaluated the %s buffer" (buffer-name)))
+  (if (use-region-p)
+      (eval-region (region-beginning) (region-end))
+    (eval-buffer))
+  (message "Evaluated the %s%s buffer"
+           (if (use-region-p) "region in the " "")
+           (buffer-name)))
 
 (defun elisp-byte-compile-file (&optional load)
   "Byte compile the file the current buffer is visiting.
```

bug#59350: 29.0.50; Eval region or buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 18 Nov 2022 09:44:39 +0200
> 
> 'elisp-eval-buffer' is a nice new command that evaluates the current buffer
> with a convenient keybinding 'C-c C-e'.  It would be nicer also to react on
> the currently active region:

That'd be confusing, IMO, given that the command says "buffer" in its
name.  We usually do this the other way around: eval-region should
work on the entire accessible portion of the buffer if the mark is not
set.  But note that eval-region doesn't need the region to be active,
so there be dragons.

bug#59350: 29.0.50; Eval region or buffer

[Juri Linkov]

>> 'elisp-eval-buffer' is a nice new command that evaluates the current buffer
>> with a convenient keybinding 'C-c C-e'.  It would be nicer also to react on
>> the currently active region:
>
> That'd be confusing, IMO, given that the command says "buffer" in its
> name.  We usually do this the other way around: eval-region should
> work on the entire accessible portion of the buffer if the mark is not
> set.  But note that eval-region doesn't need the region to be active,
> so there be dragons.

Maybe then better to rename it to 'elisp-eval-region-or-buffer',
then explain in the docstring it's for Transient Mark only.

bug#59350: 29.0.50; Eval region or buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 59350@debbugs.gnu.org
> Date: Sat, 19 Nov 2022 20:59:19 +0200
> 
> >> 'elisp-eval-buffer' is a nice new command that evaluates the current buffer
> >> with a convenient keybinding 'C-c C-e'.  It would be nicer also to react on
> >> the currently active region:
> >
> > That'd be confusing, IMO, given that the command says "buffer" in its
> > name.  We usually do this the other way around: eval-region should
> > work on the entire accessible portion of the buffer if the mark is not
> > set.  But note that eval-region doesn't need the region to be active,
> > so there be dragons.
> 
> Maybe then better to rename it to 'elisp-eval-region-or-buffer',
> then explain in the docstring it's for Transient Mark only.

Rename elisp-eval-buffer, you mean?  That's a possibility, yes.

bug#59350: 29.0.50; Eval region or buffer

[Juri Linkov]

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

>> Maybe then better to rename it to 'elisp-eval-region-or-buffer',
>> then explain in the docstring it's for Transient Mark only.
>
> Rename elisp-eval-buffer, you mean?  That's a possibility, yes.

This patch renames elisp-eval-buffer:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: elisp-eval-region-or-buffer.patch --]
[-- Type: text/x-diff, Size: 2150 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 8a34afe8d2..9345cb06f5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1464,8 +1464,8 @@ This command visits the file on the current line with EWW.
 ** Elisp
 
 ---
-*** New command 'elisp-eval-buffer' (bound to 'C-c C-e').
-This command evals the forms in the current buffer.
+*** New command 'elisp-eval-region-or-buffer' (bound to 'C-c C-e').
+This command evals the forms in the active region or in the whole buffer.
 
 ---
 *** New commands 'elisp-byte-compile-file' and 'elisp-byte-compile-buffer'.
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 44c8ca7cb9..7c470de195 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -52,7 +52,7 @@ emacs-lisp-mode-map
   :parent lisp-mode-shared-map
   "M-TAB" #'completion-at-point
   "C-M-x" #'eval-defun
-  "C-c C-e" #'elisp-eval-buffer
+  "C-c C-e" #'elisp-eval-region-or-buffer
   "C-c C-f" #'elisp-byte-compile-file
   "C-c C-b" #'elisp-byte-compile-buffer
   "C-M-q" #'indent-pp-sexp)
@@ -1234,7 +1234,7 @@ lisp-interaction-mode-map
   :parent lisp-mode-shared-map
   "C-M-x" #'eval-defun
   "C-M-q" #'indent-pp-sexp
-  "C-c C-e" #'elisp-eval-buffer
+  "C-c C-e" #'elisp-eval-region-or-buffer
   "C-c C-b" #'elisp-byte-compile-buffer
   "M-TAB" #'completion-at-point
   "C-j"   #'eval-print-last-sexp)
@@ -2212,11 +2212,17 @@ elisp-flymake--batch-compile-for-flymake
     (terpri)
     (pp collected)))
 
-(defun elisp-eval-buffer ()
-  "Evaluate the forms in the current buffer."
+(defun elisp-eval-region-or-buffer ()
+  "Evaluate the forms in the active region or the whole current buffer.
+In Transient Mark mode when the mark is active, call `eval-region'.
+Otherwise, call `eval-buffer'."
   (interactive)
-  (eval-buffer)
-  (message "Evaluated the %s buffer" (buffer-name)))
+  (if (use-region-p)
+      (eval-region (region-beginning) (region-end))
+    (eval-buffer))
+  (message "Evaluated the %s%s buffer"
+           (if (use-region-p) "region in the " "")
+           (buffer-name)))
 
 (defun elisp-byte-compile-file (&optional load)
   "Byte compile the file the current buffer is visiting.

bug#59350: 29.0.50; Eval region or buffer

[Juri Linkov]

>>> Maybe then better to rename it to 'elisp-eval-region-or-buffer',
>>> then explain in the docstring it's for Transient Mark only.
>>
>> Rename elisp-eval-buffer, you mean?  That's a possibility, yes.
>
> This patch renames elisp-eval-buffer:

The renamed command is pushed now, and closed.

bug#59350: 29.0.50; Eval region or buffer

[Drew Adams]

> 'elisp-eval-buffer' is a nice new command that evaluates the current
> buffer with a convenient keybinding 'C-c C-e'.  It would be nicer also to react
> on the currently active region:

I think the more usual (conventional?) naming is to
name the function "*-region", not "*-buffer", and
to have the doc string say that if the region isn't
active (or isn't active and nonempty, or whatever)
then the limits used are `point-min' and `point-max'
(IOW, the possibly-restricted buffer).

The region case is more general (arbitrary limits),
even if it's more particular if applied only when
active (and maybe nonempty).

Another usual (conventional?) naming, depending on
the context, can be just `*', explaining in the doc
string that the function applies to the region if
active (or...), and otherwise to the buffer.

Commands named `*-buffer' tend to act on the full
buffer (possibly restricted by narrowing), whether
or not the region is active.

I don't claim this is a rule, but I think it's
fairly common practice.

It's also fairly common to have separate commands
- or at least functions - for the region and the
buffer.

(I could be wrong.)