unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Ibuffer: w and B default to buffer at current line
@ 2016-09-14  5:35 Tino Calancha
  2016-09-14  6:41 ` John Wiegley
  0 siblings, 1 reply; 27+ messages in thread
From: Tino Calancha @ 2016-09-14  5:35 UTC (permalink / raw)
  To: Emacs developers; +Cc: tino.calancha

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


Hi,

i would like to apply following two patches.

I)
* lisp/ibuffer.el (ibuffer-get-marked-buffers):
Add an optional parameter BUFFER-AT-LINE: if non-nil and
there are no marked buffers, return the buffer at current line.
II)
Ibuffer: 'w' and 'B' default to the buffer at current line.
This is similar as dired-copy-filename-as-kill.
Before this patch, when there are no marked buffers
these commands signal an error:
"No buffers marked; use ’m’ to mark a buffer".
Append to kill ring when last command was a kill-region,
like in Dired.

Please, let me know if you have any concern about these patches.
Regards,
Tino

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 2ede73551f974022ef30c9198940a8ec10d6dd5a Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 14 Sep 2016 13:46:03 +0900
Subject: [PATCH 1/2] ibuffer-get-marked-buffers: Default to buffer at 
current
  line

* lisp/ibuffer.el (ibuffer-get-marked-buffers):
Added optional arg BUFFER-AT-LINE: if non-nil and
there are no marked buffers, then return a list with
the buffer at current line.
(lisp/ibuffer.el): Use ibuffer-get-marked-buffers with non-nil argument.
* lisp/ibuf-ext.el (ibuffer-diff-with-file): Idem.
---
  lisp/ibuf-ext.el |  4 +---
  lisp/ibuffer.el  | 26 ++++++++++++++++----------
  2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index f93957e..c4e37cb 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -1402,9 +1402,7 @@ ibuffer-diff-with-file
  This requires the external program \"diff\" to be in your `exec-path'."
    (interactive)
    (require 'diff)
-  (let ((marked-bufs (ibuffer-get-marked-buffers)))
-    (when (null marked-bufs)
-      (setq marked-bufs (list (ibuffer-current-buffer t))))
+  (let ((marked-bufs (ibuffer-get-marked-buffers 'buffer-at-line)))
      (with-current-buffer (get-buffer-create "*Ibuffer Diff*")
        (setq buffer-read-only nil)
        (buffer-disable-undo (current-buffer))
diff --git a/lisp/ibuffer.el b/lisp/ibuffer.el
index 8e24629..3915a30 100644
--- a/lisp/ibuffer.el
+++ b/lisp/ibuffer.el
@@ -1143,9 +1143,7 @@ ibuffer-do-view-horizontally
    (ibuffer-do-view-1 (if other-frame 'other-frame 'horizontally)))

  (defun ibuffer-do-view-1 (type)
-  (let ((marked-bufs (ibuffer-get-marked-buffers)))
-    (when (null marked-bufs)
-      (setq marked-bufs (list (ibuffer-current-buffer t))))
+  (let ((marked-bufs (ibuffer-get-marked-buffers 'buffer-at-line)))
      (unless (and (eq type 'other-frame)
  		 (not ibuffer-expert)
  		 (> (length marked-bufs) 3)
@@ -2020,13 +2018,21 @@ ibuffer-map-lines
  	(ibuffer-forward-line 0)
  	(ibuffer-forward-line (1- target-line-offset))))))

-(defun ibuffer-get-marked-buffers ()
-  "Return a list of buffer objects currently marked."
-  (delq nil
-	(mapcar (lambda (e)
-		  (when (eq (cdr e) ibuffer-marked-char)
-		    (car e)))
-		(ibuffer-current-state-list))))
+(defun ibuffer-get-marked-buffers (&optional buffer-at-line)
+  "Return a list of buffer objects currently marked.
+If optional arg BUFFER-AT-LINE is non-nil and there are
+no marked buffers, then return a list with the buffer
+at current line, if any."
+  (let ((buffers
+         (delq nil
+               (mapcar (lambda (e)
+                         (when (eq (cdr e) ibuffer-marked-char)
+                           (car e)))
+                       (ibuffer-current-state-list)))))
+    (when (and (null buffers)
+               buffer-at-line
+               (ibuffer-current-buffer))
+      (setq buffers (list (ibuffer-current-buffer)))) buffers))

  (defun ibuffer-current-state-list (&optional pos)
    "Return a list like (BUF . MARK) of all buffers in an ibuffer.
-- 
2.9.3

From 2372eec587229008bcb7babaffc383dd750ff8ac Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 14 Sep 2016 14:14:20 +0900
Subject: [PATCH 2/2] Ibuffer: 'w' and 'B' default to buffer at current 
line

* lisp/ibuf-ext.el (ibuffer-copy-buffername-as-kill):
If there are not marked buffers, use buffer at current line
without prompting the user.  Use ibuffer-get-marked-buffers
instead of ibuffer-map-marked-lines.
Append to kill ring when last command was a kill-region.
(ibuffer-copy-filename-as-kill): Idem.
Simplify the code.
Use ibuffer-buffer-file-name instead of buffer-file-name to
include buffers in Dired mode.
---
  lisp/ibuf-ext.el | 70 
+++++++++++++++++++++++---------------------------------
  1 file changed, 29 insertions(+), 41 deletions(-)

diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index c4e37cb..29a9349 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -1430,37 +1430,27 @@ ibuffer-copy-filename-as-kill

  You can then feed the file name(s) to other commands with \\[yank]."
    (interactive "p")
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((result "")
-	  (type (cond ((or (null arg) (zerop arg))
-		       'full)
-		      ((= arg 4)
-		       'relative)
-		      (t
-		       'name))))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-	   (setq result
-                 (concat result
-                         (let ((name (buffer-file-name buf)))
-                           (cond (name
-                                  (concat
-                                   (pcase type
-                                     (`full
-                                      name)
-                                     (`relative
-                                      (file-relative-name
-                                       name (or ibuffer-default-directory
-                                                default-directory)))
-                                     (_
-                                      (file-name-nondirectory name))) " 
"))
-                                 (t "")))))))
-      (when (not (zerop (length result)))
-        (setq result
-              (substring result 0 -1)))
-      (kill-new result)
-      (message "%s" result))))
+  (let* ((file-names
+          (mapcar
+           (lambda (buf)
+             (let ((name (with-current-buffer buf
+                           (ibuffer-buffer-file-name))))
+               (if (null name)
+                   ""
+                 (cond ((or (null arg) (zerop arg)) name)
+                       ((= arg 4)
+                        (file-relative-name
+                         name (or ibuffer-default-directory
+                                  default-directory)))
+                       (t (file-name-nondirectory name))))))
+           (ibuffer-get-marked-buffers 'buffer-at-line)))
+         (string
+          (mapconcat 'identity (delete "" file-names) " ")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  ;;;###autoload
  (defun ibuffer-copy-buffername-as-kill ()
@@ -1468,16 +1458,14 @@ ibuffer-copy-buffername-as-kill
  The names are separated by a space.
  You can then feed the file name(s) to other commands with \\[yank]."
    (interactive)
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((res ""))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-           (setq res (concat res (buffer-name buf) " "))))
-      (when (not (zerop (length res)))
-        (setq res (substring res 0 -1)))
-      (kill-new res)
-      (message res))))
+  (let ((string
+         (mapconcat #'buffer-name
+                    (ibuffer-get-marked-buffers 'buffer-at-line) " ")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  (defun ibuffer-mark-on-buffer (func &optional ibuffer-mark-on-buffer-mark 
group)
    (let ((count
-- 
2.9.3


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.21.5)
  of 2016-09-14
Repository revision: 3b9cbacf6110daf7cb2a838e28afef1e4f5ff831

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-14  5:35 Ibuffer: w and B default to buffer at current line Tino Calancha
@ 2016-09-14  6:41 ` John Wiegley
  2016-09-14  7:21   ` Tino Calancha
  2016-09-17 18:22   ` Ibuffer: w and B default to buffer at current line Tino Calancha
  0 siblings, 2 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-14  6:41 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Emacs developers

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

>>>>> "TC" == Tino Calancha <tino.calancha@gmail.com> writes:

TC> * lisp/ibuffer.el (ibuffer-get-marked-buffers):
TC> Add an optional parameter BUFFER-AT-LINE: if non-nil and
TC> there are no marked buffers, return the buffer at current line.

Hi Tino,

I don't like this part of your proposed change. The name of the function is
`ibuffer-get-marked-buffers'. Adding a new argument to change the meaning of
the function is not a good idea.

Instead, there should be a new function `ibuffer-get-buffer-at-point' or some
such, to reflect the meaning you wish to express. There may even already be
such a function, I'm not all that familiar with ibuffer.el.

TC> Ibuffer: 'w' and 'B' default to the buffer at current line.
TC> This is similar as dired-copy-filename-as-kill.
TC> Before this patch, when there are no marked buffers
TC> these commands signal an error:

This part sounds reasonable to me. I like the idea of defaulting behavior when
there is no current selection or region.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-14  6:41 ` John Wiegley
@ 2016-09-14  7:21   ` Tino Calancha
  2016-09-14 14:08     ` Drew Adams
  2016-09-15 22:05     ` John Wiegley
  2016-09-17 18:22   ` Ibuffer: w and B default to buffer at current line Tino Calancha
  1 sibling, 2 replies; 27+ messages in thread
From: Tino Calancha @ 2016-09-14  7:21 UTC (permalink / raw)
  To: jwiegley; +Cc: tino.calancha, Emacs developers



Thanks for your comments John!

On Tue, 13 Sep 2016, John Wiegley wrote:

>>>>>> "TC" == Tino Calancha <tino.calancha@gmail.com> writes:
>
> TC> * lisp/ibuffer.el (ibuffer-get-marked-buffers):
> TC> Add an optional parameter BUFFER-AT-LINE: if non-nil and
> TC> there are no marked buffers, return the buffer at current line.
>
> Hi Tino,
>
> I don't like this part of your proposed change. The name of the function is
> `ibuffer-get-marked-buffers'. Adding a new argument to change the meaning of
> the function is not a good idea.
If you compare the first patch with `dired-get-marked-files' (d-g-m-f)
you might change your mind:
(d-g-m-f) returns a list with the file at point when there are no 
marked files.  The first patch follow similar idea.

> Instead, there should be a new function `ibuffer-get-buffer-at-point' or some
> such, to reflect the meaning you wish to express. There may even already be
> such a function, I'm not all that familiar with ibuffer.el.
There is one, ibuffer-current-buffer.
My point is that patch1 allow me to implement 
'ibuffer-copy-filename-as-kill' (i-c-f-a-k) very similar as 
'dired-copy-filename-as-kill' (d-c-f-a-k).  That is nice to me.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
  2016-09-14  7:21   ` Tino Calancha
@ 2016-09-14 14:08     ` Drew Adams
  2016-09-15 22:05     ` John Wiegley
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2016-09-14 14:08 UTC (permalink / raw)
  To: Tino Calancha, jwiegley; +Cc: Emacs developers

> > I don't like this part of your proposed change. The name of the
> > function is `ibuffer-get-marked-buffers'. Adding a new argument
> > to change the meaning of the function is not a good idea.
>
> If you compare the first patch with `dired-get-marked-files' (d-g-m-f)
> you might change your mind:
> (d-g-m-f) returns a list with the file at point when there are no
> marked files.  The first patch follow similar idea.

I was going to say something similar to Tino.  The same is true
of Dired commands generally - e.g., all of the `dired-do-*'
commands act on the file/dir of the current line if none are
marked: A, B, C, D, G, H, L, M, O, P, Q, R, S, T, U, X, and Z.

(And a few of them correspond to single-file commands for the
current-line's file/dir: c, d, and u.  In Dired+ this is a bit
more the case: e/E, c/C, d/D, f/F, i/I, j/J, u/U, and z/Z)

I have no opinion about the proposal for Ibuffer.  But it
sounds like it is applying a longstanding UI approach that
is used in Dired - one that I think is sound and useful.

> > Instead, there should be a new function `ibuffer-get-buffer-at-point'
> > or some such, to reflect the meaning you wish to express.
> > There may even already be such a function, I'm not all that
> > familiar with ibuffer.el.
>
> There is one, ibuffer-current-buffer.
> My point is that patch1 allow me to implement
> 'ibuffer-copy-filename-as-kill' (i-c-f-a-k) very similar as
> 'dired-copy-filename-as-kill' (d-c-f-a-k).  That is nice to me.

Again, Dired offers separate commands that apply only to the
file/dir of the current line.  They are on menus, for example.
Keys are generally not needed for them, precisely because of
the if-none-marked-then-act-on-this-one behavior.

The point of this design is to let you use the same key to act
on a single file/buffer.  In addition, in Dired you can provide
a numeric prefix arg to act on the next (or previous) N files.
N defaults to 1, of course, which provides the this-line's-file
behavior.

(In Emacs there is more than one way to skin the cat...
This is a plus, not a minus.)



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-14  7:21   ` Tino Calancha
  2016-09-14 14:08     ` Drew Adams
@ 2016-09-15 22:05     ` John Wiegley
  2016-09-16  6:40       ` Eli Zaretskii
       [not found]       ` <<83intw5our.fsf@gnu.org>
  1 sibling, 2 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-15 22:05 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Emacs developers

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

>>>>> Tino Calancha <tino.calancha@gmail.com> writes:

> If you compare the first patch with `dired-get-marked-files' (d-g-m-f) you
> might change your mind: (d-g-m-f) returns a list with the file at point when
> there are no marked files. The first patch follow similar idea.

I understand dired may also be taking this bad approach, but that's not a good
reason to continue the practice.

The end goal you're trying to achieve is to make a keystroke more intuitive.
That's a great goal. What it sounds like to me is that we need another layer
above ibuffer-get-marked-files: a command that, if there are marked buffers,
calls ibuffer-get-marked-files, and if there are no marked buffers, calls
ibuffer-current-buffer. Then it's clear to the reader what the command means,
and what is happening, and each function has exactly one job to do.

Monkey-patching Emacs to make single-purpose functions more magical by adding
special arguments -- or special meanings to arguments -- will burden us with
unnecessary technical debt in the future. I don't like this practice, and I'm
unmoved by "prior art" as a justification.

That said, again, I'm 100% in support of the end goal you want to achieve; I
just want to get there in a more principled way. Functions should do what they
say, and not have lots of alternative functionality baked into them because
it's a convenient hack today.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-15 22:05     ` John Wiegley
@ 2016-09-16  6:40       ` Eli Zaretskii
       [not found]       ` <<83intw5our.fsf@gnu.org>
  1 sibling, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-16  6:40 UTC (permalink / raw)
  To: John Wiegley; +Cc: emacs-devel, tino.calancha

> From: John Wiegley <jwiegley@gmail.com>
> Date: Thu, 15 Sep 2016 15:05:20 -0700
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> > If you compare the first patch with `dired-get-marked-files' (d-g-m-f) you
> > might change your mind: (d-g-m-f) returns a list with the file at point when
> > there are no marked files. The first patch follow similar idea.
> 
> I understand dired may also be taking this bad approach, but that's not a good
> reason to continue the practice.

But it isn't just Dired.  Having the marked items default to the
current one is a very frequent idiom in Emacs modes that present lists
of actionable items.  It is also consistent with what GUI file
managers out there do.

> Functions should do what they say, and not have lots of alternative
> functionality baked into them because it's a convenient hack today.

There's a clash here between this goal, and the goal of being
DWIMish.  One way of resolving the contradiction is to have the info
in the function's name, and if all else fails, in the doc string.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
       [not found]       ` <<83intw5our.fsf@gnu.org>
@ 2016-09-16 14:53         ` Drew Adams
  2016-09-17 16:30           ` John Wiegley
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2016-09-16 14:53 UTC (permalink / raw)
  To: Eli Zaretskii, John Wiegley; +Cc: tino.calancha, emacs-devel

> > > If you compare the first patch with `dired-get-marked-files' (d-g-m-f)
> > > you might change your mind: (d-g-m-f) returns a list with the file at
> > > point when there are no marked files. The first patch follow similar 
> > > idea.
> >
> > I understand dired may also be taking this bad approach, but that's not
> > a good reason to continue the practice.
> 
> But it isn't just Dired.  Having the marked items default to the
> current one is a very frequent idiom in Emacs modes that present lists
> of actionable items.  It is also consistent with what GUI file
> managers out there do.
> 
> > Functions should do what they say, and not have lots of alternative
> > functionality baked into them because it's a convenient hack today.
> 
> There's a clash here between this goal, and the goal of being
> DWIMish.

1. Distinguish (a) behavior for users from (b) its implementation.

I started to reply in a similar vein to Eli.  On rereading
John's mail I saw that he tried to separate what the behavior
is for users from how the behavior is implemented, and his
argument seems to be only about the latter (although it was
not made as clearly as it might have been, IMO).

IOW, I don't think John is arguing against the UI design of
letting a key such as `C' or `F' in Dired have different
behaviors depending on the use of a prefix arg.

I think his only argument is about how that (good, not bad)
behavior is implemented.  He argues that there should be a
separate function for acting on only the current line's
file and not the marked files.

And as I mentioned, it is often the case that there _is_
such a separate function.  And it is often a command.  But
it is typically not bound to a key - it is used only for
a menu-item binding, since the general `C', `F', etc. key
does double-duty.

There is generally no need to bind a key sequence to such
a this-file command.  Put differently, there already _is_
a key sequence that gives you that behavior: a prefix arg
(`M-1') followed by the multiple-files key: act on only
one file - this one.

So I think this is some (not much) ado about nothing.
The UI is a sound and useful one.  And the implementation
is also not bad - it generally does not conflict with
John's request for single-purpose functions.  Specific
single-file functions are not always needed in Dired,
but when they are needed (e.g., for a menu item), they
exist.

And a utility function that accepts args that control
variant behaviors is also not necessarily a bad thing,
IMO.

No one would argue that `mapc' is bad because it is
parameterized by a function argument that determines
the outcome/behavior.  No one would outlaw it because
it violates a single-function-per-behavior principle.
Likewise, no one would propose removing `if' because
it allows for different behaviors.

The devil is in the details.  That's my critique of
John's comments - too general/vague.  To discuss
whether there is really a problem in this case, the
complaint should be specific, about the actual code,
not just couched in general software-engineering terms.

The talk of "bad approach" and "monkey-patching" was
misplaced, I think.  There is a sermon there, but I
don't think there is a sinner to be preached to -
only fellow choir members. I think that John spoke
in too-general terms, with some hand-waving.

Even if we single-purpose functions, so they satisfy
John's "principled way", there can be an advantage,
for interactive use, in combining them in _commands_
that can have multiple, alternative behaviors.  I
think John acknowledges this and even said much the
same thing.

And providing different behaviors in the same command
is not just a holdover from "prior art".  It is an
important possibility/feature that the basic design of
Emacs offers (and yes, that it has used in commands
provided by default, from Day One).

That does not mean that just any old throwing together
of behaviors in a single command is a great idea.  It
means that some commands that provide for multiple
behaviors can be good things, not bad.

IOW, one size/principle does not fit all use cases.
What justifies something like the prefix-arg behavior
of Dired commands for marked files is not the fact
that it is "prior art".  No one argued that.  That
neither justifies nor invalidates it.  What justifies
it is the judgment that the behavior is helpful for users.

That's where the argument about what behavior to
provide needs to take place: in the realm of behavior
for users.  Not just in the realm of code organization
for easy and sound maintenance.

There are cleaner & dirtier ways, and easier & harder
to maintain ways, to implement a given behavior.  The
question about the _usefulness for users_ of such Dired
(or Ibuffer) commands is the question to address first.
Questions of implementation choice should follow that
question, not vice versa.

I think (and hope) that John does not feel very different
about this.


2. As for DWIM, which Eli mentions: This is not about DWIM.

I distinguish (a) the ability for users to obtain
different, alternative behaviors from a given command
from (b) DWIM commands.

The former gives the user full control.

A DWIM command is some programmer's idea of what would
be the right behavior within a given context.  It is
the command (code), not the user, that is in control.
The behavior choice is essentially made at coding time.
It is not made by the user, at runtime.

That does not mean that DWIM is always stupid, bad, or
misguided.  It means only that it is something different
from a user choosing to provide a given prefix arg, to
get a particular member of a command's set of behaviors.

In general, I'm strongly in favor of commands providing
multiple behaviors, which a user can choose from.  What
is important is that (a) the behaviors somehow belong
together in the same command, (b) the default behavior
is the common one, and (c) the doc makes clear what the
behaviors are and how to obtain them.

To me, Dired's `dired-do-*' commands (`C', `F', etc.)
are well designed.  The alternative behaviors collected
in the same command belong together; the commonly used
behavior is the default; and the doc about them is clear
enough.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-16 14:53         ` Drew Adams
@ 2016-09-17 16:30           ` John Wiegley
  2016-09-17 17:21             ` Eli Zaretskii
       [not found]             ` <<83zin630i9.fsf@gnu.org>
  0 siblings, 2 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-17 16:30 UTC (permalink / raw)
  To: Drew Adams; +Cc: Eli Zaretskii, tino.calancha, emacs-devel

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

>>>>> Drew Adams <drew.adams@oracle.com> writes:

> I started to reply in a similar vein to Eli. On rereading John's mail I saw
> that he tried to separate what the behavior is for users from how the
> behavior is implemented, and his argument seems to be only about the latter
> (although it was not made as clearly as it might have been, IMO).

> IOW, I don't think John is arguing against the UI design of letting a key
> such as `C' or `F' in Dired have different behaviors depending on the use of
> a prefix arg.

> I think his only argument is about how that (good, not bad) behavior is
> implemented. He argues that there should be a separate function for acting
> on only the current line's file and not the marked files.

Hi Drew, you've summed up very well the point I was trying to convey. My
apologies if any hand-waving was perceived; I'd be happy to construct code
examples on any point, if that is needed.

The reason for my comment is that the original patch requested adding a new
argument to an existing function, in order to special-case the behavior of
that function in a way that does not fit with its current name and purpose.
I'd rather have two functions and a new command that calls the appropriate
function based on its context of use.

What I to avoid is coding special behavior into existing functions -- *in the
implementation* -- simply because it is convenient. It still bothers me that
`call-process-region' does this, for example, with its many and varied
meanings of the "START" argument -- some of which relate neither to the
meaning of "start", nor of "region".

When it comes to UI, I'm in complete agreement with Eli: I love DWIM behavior,
and think this is a virtue of Emacs, not a vice in any way.

So yes, I'm just thinking about the code: modularity, ease of maintenance,
clarity of purpose. These may be not be terribly specific statements, but they
are the motive for my comments.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 16:30           ` John Wiegley
@ 2016-09-17 17:21             ` Eli Zaretskii
  2016-09-17 21:35               ` John Wiegley
       [not found]             ` <<83zin630i9.fsf@gnu.org>
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-17 17:21 UTC (permalink / raw)
  To: John Wiegley; +Cc: tino.calancha, drew.adams, emacs-devel

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org,  tino.calancha@gmail.com
> Date: Sat, 17 Sep 2016 09:30:00 -0700
> 
> When it comes to UI, I'm in complete agreement with Eli: I love DWIM behavior,
> and think this is a virtue of Emacs, not a vice in any way.

If DWIM is okay in the UI, then functions that behave in support of
that UI should also be okay.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-14  6:41 ` John Wiegley
  2016-09-14  7:21   ` Tino Calancha
@ 2016-09-17 18:22   ` Tino Calancha
  2016-09-26 12:08     ` Tino Calancha
  1 sibling, 1 reply; 27+ messages in thread
From: Tino Calancha @ 2016-09-17 18:22 UTC (permalink / raw)
  To: John Wiegley; +Cc: Eli Zaretskii, tino.calancha, drew.adams, Emacs developers



On Tue, 13 Sep 2016, John Wiegley wrote:

>>>>>> "TC" == Tino Calancha <tino.calancha@gmail.com> writes:
>
> TC> * lisp/ibuffer.el (ibuffer-get-marked-buffers):
> TC> Add an optional parameter BUFFER-AT-LINE: if non-nil and
> TC> there are no marked buffers, return the buffer at current line.
>
> Hi Tino,
>
> I don't like this part of your proposed change. The name of the function is
> `ibuffer-get-marked-buffers'. Adding a new argument to change the meaning of
> the function is not a good idea.

Hi John,

After Drew comments i realized something useful was still missing in
my previous patch: a non-zero integer ARG should use the next (or previous
if ARG is negative) buffers, regardless if there are marked buffers.
That is the behaviour in `dired-copy-filename-as-kill'.  I want to support
the same in `ibuffer-copy-filename-as-kill' (i-c-f-a-k) and
`ibuffer-copy-buffername-as-kill' (i-c-b-a-k).

I have prepared to patches `D' and `W': the former follows DWIM, the later
follows your suggestion.

`D':
* 2 files changed, 65 insertions(+), 58 deletions(-)

`W':
* 2 files changed, 52 insertions(+), 46 deletions(-)

I don't have any preference for one or the other.  Both provide the same
behaviour, so i am fine to push the one that people here prefer.

Best regards,
Tino

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
patch `D'
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From a06c406a5cd7da5c645c5365229144525fe55783 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 18 Sep 2016 02:40:08 +0900
Subject: [PATCH] Ibuffer: 'w' and 'B' default to buffer at current line

See discussion in:
https://lists.gnu.org/archive/html/emacs-devel/2016-09/msg00384.html
* lisp/ibuffer.el (ibuffer--near-buffers): New defun;
return buffers near point.
(ibuffer-get-marked-buffers): Use it.
Add argument ARG; if a non-zero integer, return next ARG files.
Add argument BUFFER-AT-LINE; if non-nil, and there are no
marked files, return the buffer at current line.
(ibuffer-do-view-horizontally): Call ibuffer-get-marked-buffers
with non-nil second argument.
* lisp/ibuf-ext.el (ibuffer-diff-with-file): Idem.
(ibuffer-copy-buffername-as-kill):
Add argument ARG; if an integer, return next ARG files.  Otherwise,
return the marked files.  If there are not marked buffers, use buffer
at current line without prompting the user.
Use ibuffer-get-marked-buffers instead of ibuffer-map-marked-lines.
Append to kill ring when last command was a kill-region.
(ibuffer-copy-filename-as-kill): Idem.
Simplify the code.
Use ibuffer-buffer-file-name instead of buffer-file-name to
include buffers in Dired mode.
---
  lisp/ibuf-ext.el | 84 
+++++++++++++++++++++++---------------------------------
  lisp/ibuffer.el  | 39 ++++++++++++++++++++------
  2 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index f93957e..ad5298a 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -1402,9 +1402,7 @@ ibuffer-diff-with-file
  This requires the external program \"diff\" to be in your `exec-path'."
    (interactive)
    (require 'diff)
-  (let ((marked-bufs (ibuffer-get-marked-buffers)))
-    (when (null marked-bufs)
-      (setq marked-bufs (list (ibuffer-current-buffer t))))
+  (let ((marked-bufs (ibuffer-get-marked-buffers nil 'buffer-at-line)))
      (with-current-buffer (get-buffer-create "*Ibuffer Diff*")
        (setq buffer-read-only nil)
        (buffer-disable-undo (current-buffer))
@@ -1420,7 +1418,7 @@ ibuffer-diff-with-file

  ;;;###autoload
  (defun ibuffer-copy-filename-as-kill (&optional arg)
-  "Copy filenames of marked buffers into the kill ring.
+  "Copy filenames of marked (or next ARG) buffers into the kill ring.

  The names are separated by a space.
  If a buffer has no filename, it is ignored.
@@ -1431,55 +1429,43 @@ ibuffer-copy-filename-as-kill
  to `ibuffer-default-directory' if non-nil, otherwise `default-directory'.

  You can then feed the file name(s) to other commands with \\[yank]."
-  (interactive "p")
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((result "")
-	  (type (cond ((or (null arg) (zerop arg))
-		       'full)
-		      ((= arg 4)
-		       'relative)
-		      (t
-		       'name))))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-	   (setq result
-                 (concat result
-                         (let ((name (buffer-file-name buf)))
-                           (cond (name
-                                  (concat
-                                   (pcase type
-                                     (`full
-                                      name)
-                                     (`relative
-                                      (file-relative-name
-                                       name (or ibuffer-default-directory
-                                                default-directory)))
-                                     (_
-                                      (file-name-nondirectory name))) " 
"))
-                                 (t "")))))))
-      (when (not (zerop (length result)))
-        (setq result
-              (substring result 0 -1)))
-      (kill-new result)
-      (message "%s" result))))
+  (interactive "P")
+  (let* ((file-names
+          (mapcar
+           (lambda (buf)
+             (let ((name (with-current-buffer buf
+                           (ibuffer-buffer-file-name))))
+               (if (null name)
+                   ""
+                 (cond ((and (integerp arg) (zerop arg)) name)
+                       ((consp arg)
+                        (file-relative-name
+                         name (or ibuffer-default-directory
+                                  default-directory)))
+                       (t (file-name-nondirectory name))))))
+           (ibuffer-get-marked-buffers arg 'buffer-at-line)))
+         (string
+          (mapconcat 'identity (delete "" file-names) " ")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  ;;;###autoload
-(defun ibuffer-copy-buffername-as-kill ()
-  "Copy buffer names of marked buffers into the kill ring.
+(defun ibuffer-copy-buffername-as-kill (&optional arg)
+  "Copy buffer names of marked (or next ARG) buffers into the kill ring.
  The names are separated by a space.
  You can then feed the file name(s) to other commands with \\[yank]."
-  (interactive)
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((res ""))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-           (setq res (concat res (buffer-name buf) " "))))
-      (when (not (zerop (length res)))
-        (setq res (substring res 0 -1)))
-      (kill-new res)
-      (message res))))
+  (interactive "P")
+  (let ((string
+         (mapconcat #'buffer-name
+                    (ibuffer-get-marked-buffers arg 'buffer-at-line) " 
")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  (defun ibuffer-mark-on-buffer (func &optional ibuffer-mark-on-buffer-mark 
group)
    (let ((count
diff --git a/lisp/ibuffer.el b/lisp/ibuffer.el
index 0336f1d..859b595 100644
--- a/lisp/ibuffer.el
+++ b/lisp/ibuffer.el
@@ -1143,9 +1143,7 @@ ibuffer-do-view-horizontally
    (ibuffer-do-view-1 (if other-frame 'other-frame 'horizontally)))

  (defun ibuffer-do-view-1 (type)
-  (let ((marked-bufs (ibuffer-get-marked-buffers)))
-    (when (null marked-bufs)
-      (setq marked-bufs (list (ibuffer-current-buffer t))))
+  (let ((marked-bufs (ibuffer-get-marked-buffers nil 'buffer-at-line)))
      (unless (and (eq type 'other-frame)
  		 (not ibuffer-expert)
  		 (> (length marked-bufs) 3)
@@ -2022,13 +2020,36 @@ ibuffer-map-lines
  	(ibuffer-forward-line 0)
  	(ibuffer-forward-line (1- target-line-offset))))))

-(defun ibuffer-get-marked-buffers ()
-  "Return a list of buffer objects currently marked."
+;; Return buffers around current line.
+(defun ibuffer--near-buffers (n)
    (delq nil
-	(mapcar (lambda (e)
-		  (when (eq (cdr e) ibuffer-marked-char)
-		    (car e)))
-		(ibuffer-current-state-list))))
+        (mapcar
+         (lambda (x)
+           (car (get-text-property
+                 (line-beginning-position (if (natnump n) x (- (1- x))))
+                 'ibuffer-properties)))
+         (number-sequence 1 (abs n)))))
+
+(defun ibuffer-get-marked-buffers (&optional arg buffer-at-line)
+  "Return a list of buffer objects currently marked.
+Optional argument ARG, if non-nil, specifies buffers near
+ point instead of marked buffers.  It usually comes from the prefix
+ argument.
+  If ARG is an integer, use the next ARG files.
+If no files are marked, and BUFFER-AT-LINE is non-nil return the buffer 
at
+current line."
+  (let ((buffers
+         (cond ((and (integerp arg) (not (zerop arg)))
+                (ibuffer--near-buffers arg))
+               (t
+                (delq nil
+                      (mapcar (lambda (e)
+                                (when (eq (cdr e) ibuffer-marked-char)
+                                  (car e)))
+                              (ibuffer-current-state-list)))))))
+    (when (and (null buffers)
+               buffer-at-line)
+      (setq buffers (list (ibuffer-current-buffer 'live)))) buffers))

  (defun ibuffer-current-state-list (&optional pos)
    "Return a list like (BUF . MARK) of all buffers in an ibuffer.
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
patch `W'
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 179f608832c2de1fbbba11d4bb1b0c9064d73d2f Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 18 Sep 2016 02:42:20 +0900
Subject: [PATCH] Ibuffer: 'w' and 'B' default to buffer at current line

See discussion in:
https://lists.gnu.org/archive/html/emacs-devel/2016-09/msg00384.html
* lisp/ibuffer.el (ibuffer--near-buffers): New defun;
return buffers near point.
* lisp/ibuf-ext.el (ibuffer-copy-buffername-as-kill): Use it.
Add argument ARG; if a non-zero integer, return next ARG buffers.
Otherwise return the marked buffers.
If there are not marked buffers, return buffer at current line
without prompting the user.
Use ibuffer-get-marked-buffers instead of ibuffer-map-marked-lines.
Append to kill ring when last command was a kill-region.
(ibuffer-copy-filename-as-kill): Idem.
Simplify the code.
Use ibuffer-buffer-file-name instead of buffer-file-name to
include buffers in Dired mode.
---
  lisp/ibuf-ext.el | 88 
+++++++++++++++++++++++++++-----------------------------
  lisp/ibuffer.el  | 10 +++++++
  2 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index f93957e..1918ce8 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -1420,7 +1420,7 @@ ibuffer-diff-with-file

  ;;;###autoload
  (defun ibuffer-copy-filename-as-kill (&optional arg)
-  "Copy filenames of marked buffers into the kill ring.
+  "Copy filenames of marked (or next ARG) buffers into the kill ring.

  The names are separated by a space.
  If a buffer has no filename, it is ignored.
@@ -1431,55 +1431,51 @@ ibuffer-copy-filename-as-kill
  to `ibuffer-default-directory' if non-nil, otherwise `default-directory'.

  You can then feed the file name(s) to other commands with \\[yank]."
-  (interactive "p")
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((result "")
-	  (type (cond ((or (null arg) (zerop arg))
-		       'full)
-		      ((= arg 4)
-		       'relative)
-		      (t
-		       'name))))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-	   (setq result
-                 (concat result
-                         (let ((name (buffer-file-name buf)))
-                           (cond (name
-                                  (concat
-                                   (pcase type
-                                     (`full
-                                      name)
-                                     (`relative
-                                      (file-relative-name
-                                       name (or ibuffer-default-directory
-                                                default-directory)))
-                                     (_
-                                      (file-name-nondirectory name))) " 
"))
-                                 (t "")))))))
-      (when (not (zerop (length result)))
-        (setq result
-              (substring result 0 -1)))
-      (kill-new result)
-      (message "%s" result))))
+  (interactive "P")
+  (let* ((buffers (cond ((and (integerp arg) (not (zerop arg)))
+                         (ibuffer--near-buffers arg))
+                        (t
+                         (or (ibuffer-get-marked-buffers)
+                             (list (ibuffer-current-buffer))))))
+         (file-names
+          (mapcar
+           (lambda (buf)
+             (let ((name (with-current-buffer buf
+                           (ibuffer-buffer-file-name))))
+               (if (null name)
+                   ""
+                 (cond ((and (integerp arg) (zerop arg)) name)
+                       ((consp arg)
+                        (file-relative-name
+                         name (or ibuffer-default-directory
+                                  default-directory)))
+                       (t (file-name-nondirectory name))))))
+           buffers))
+         (string
+          (mapconcat 'identity (delete "" file-names) " ")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  ;;;###autoload
-(defun ibuffer-copy-buffername-as-kill ()
-  "Copy buffer names of marked buffers into the kill ring.
+(defun ibuffer-copy-buffername-as-kill (&optional arg)
+  "Copy buffer names of marked (or next ARG) buffers into the kill ring.
  The names are separated by a space.
  You can then feed the file name(s) to other commands with \\[yank]."
-  (interactive)
-  (if (zerop (ibuffer-count-marked-lines))
-      (message "No buffers marked; use 'm' to mark a buffer")
-    (let ((res ""))
-      (ibuffer-map-marked-lines
-       #'(lambda (buf _mark)
-           (setq res (concat res (buffer-name buf) " "))))
-      (when (not (zerop (length res)))
-        (setq res (substring res 0 -1)))
-      (kill-new res)
-      (message res))))
+  (interactive "P")
+  (let* ((buffers (cond ((and (integerp arg) (not (zerop arg)))
+                         (ibuffer--near-buffers arg))
+                        (t
+                         (or (ibuffer-get-marked-buffers)
+                             (list (ibuffer-current-buffer))))))
+         (string (mapconcat #'buffer-name buffers " ")))
+    (unless (string= string "")
+      (if (eq last-command 'kill-region)
+          (kill-append string nil)
+        (kill-new string))
+      (message "%s" string))))

  (defun ibuffer-mark-on-buffer (func &optional ibuffer-mark-on-buffer-mark 
group)
    (let ((count
diff --git a/lisp/ibuffer.el b/lisp/ibuffer.el
index 0336f1d..fca8ba3 100644
--- a/lisp/ibuffer.el
+++ b/lisp/ibuffer.el
@@ -2022,6 +2022,16 @@ ibuffer-map-lines
  	(ibuffer-forward-line 0)
  	(ibuffer-forward-line (1- target-line-offset))))))

+;; Return buffers around current line.
+(defun ibuffer--near-buffers (n)
+  (delq nil
+        (mapcar
+         (lambda (x)
+           (car (get-text-property
+                 (line-beginning-position (if (natnump n) x (- (1- x))))
+                 'ibuffer-properties)))
+         (number-sequence 1 (abs n)))))
+
  (defun ibuffer-get-marked-buffers ()
    "Return a list of buffer objects currently marked."
    (delq nil
-- 
2.9.3

In GNU Emacs 25.1.50.1
Repository revision: d7f0daf7ecaa7b138f7c66796e23fa5982b0a8bb




^ permalink raw reply related	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
       [not found]             ` <<83zin630i9.fsf@gnu.org>
@ 2016-09-17 18:47               ` Drew Adams
  2016-09-17 19:25                 ` Eli Zaretskii
       [not found]                 ` <<83vaxuib1p.fsf@gnu.org>
  0 siblings, 2 replies; 27+ messages in thread
From: Drew Adams @ 2016-09-17 18:47 UTC (permalink / raw)
  To: Eli Zaretskii, John Wiegley; +Cc: tino.calancha, drew.adams, emacs-devel

> > When it comes to UI, I'm in complete agreement with Eli: I love DWIM
> > behavior, and think this is a virtue of Emacs, not a vice in any way.
> 
> If DWIM is okay in the UI, then functions that behave in support of
> that UI should also be okay.

Yes, of course.  But as I pointed out, the case at hand has
nothing to do with DWIM.  Zero.  It is simply about commands
that let a user choose an alternative behavior with a prefix
arg.

The command is not guessing anything.  It is not using particular
information about the specific current context to choose among
possible behaviors.  The user is explicitly specifying the
behavior to use.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 18:47               ` Drew Adams
@ 2016-09-17 19:25                 ` Eli Zaretskii
       [not found]                 ` <<83vaxuib1p.fsf@gnu.org>
  1 sibling, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-17 19:25 UTC (permalink / raw)
  To: Drew Adams; +Cc: jwiegley, tino.calancha, drew.adams, emacs-devel

> Date: Sat, 17 Sep 2016 11:47:35 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: drew.adams@oracle.com, emacs-devel@gnu.org, tino.calancha@gmail.com
> 
> > > When it comes to UI, I'm in complete agreement with Eli: I love DWIM
> > > behavior, and think this is a virtue of Emacs, not a vice in any way.
> > 
> > If DWIM is okay in the UI, then functions that behave in support of
> > that UI should also be okay.
> 
> Yes, of course.  But as I pointed out, the case at hand has
> nothing to do with DWIM.

I disagree.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
       [not found]                 ` <<83vaxuib1p.fsf@gnu.org>
@ 2016-09-17 19:33                   ` Drew Adams
  2016-09-18 14:29                     ` Eli Zaretskii
       [not found]                   ` <<d33a60f5-b8b6-4637-b3e6-ea1b09d98f85@default>
  1 sibling, 1 reply; 27+ messages in thread
From: Drew Adams @ 2016-09-17 19:33 UTC (permalink / raw)
  To: Eli Zaretskii, Drew Adams; +Cc: jwiegley, tino.calancha, emacs-devel

> > > > When it comes to UI, I'm in complete agreement with Eli: I love DWIM
> > > > behavior, and think this is a virtue of Emacs, not a vice in any
> > > > way.
> > >
> > > If DWIM is okay in the UI, then functions that behave in support of
> > > that UI should also be okay.
> >
> > Yes, of course.  But as I pointed out, the case at hand has
> > nothing to do with DWIM.
> 
> I disagree.

And your reason is?

A user _explicitly specifies_ which to use, of the behaviors
the command supports.  How is that guess-what-I-mean-and-do-it?

Is `forward-char` a DWIM command, simply because it can move
different distances depending on a prefix arg?  I don't think
so, but sure, feel free to disagree.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 17:21             ` Eli Zaretskii
@ 2016-09-17 21:35               ` John Wiegley
  2016-09-17 23:26                 ` Drew Adams
  2016-09-18 14:26                 ` Eli Zaretskii
  0 siblings, 2 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-17 21:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tino.calancha, drew.adams, emacs-devel

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

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> When it comes to UI, I'm in complete agreement with Eli: I love DWIM
>> behavior, and think this is a virtue of Emacs, not a vice in any way.

> If DWIM is okay in the UI, then functions that behave in support of that UI
> should also be okay.

Since this responses surprised me a bit, I'm going to assume that I've
misunderstood somehow.

Let me give a hypothetical example: Assume for the sake of argument that
`count-lines' did not report characters as well, and that someone wished to
extend the `count-lines' command so that, given a prefix arg, it would count
characters instead of lines.

What I think should happen in this case is that a new command be created:
count-items-in-region, which by default counts lines, but with a prefix
argument counts characters. This leaves that command open to many new
behaviors in future, while `count-lines' keeps doing just what it says: count
lines.

What I would object to is adding a new argument to `count-lines', called
`characters-p', that changes the behavior of count-lines to now count
characters instead (again, remember this is hypothetical, I know that today's
`count-lines' already counts characters as well).

Just because I want DWIM from my interface, doesn't mean I need to implement
it as DWIM in my functions. I believe -- very strongly -- that each function
should do one thing and one thing well, and this one thing should be
documented well. I don't like magical functions with lots of alternative
behaviors, unless it is a command created for the purpose of magically
dispatching to other functions based on its context of use. Such magical
interface functions are quite alright in my book; but complexifying the
behavior of core functions is not.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
  2016-09-17 21:35               ` John Wiegley
@ 2016-09-17 23:26                 ` Drew Adams
  2016-09-17 23:51                   ` John Wiegley
  2016-09-18 14:26                 ` Eli Zaretskii
  1 sibling, 1 reply; 27+ messages in thread
From: Drew Adams @ 2016-09-17 23:26 UTC (permalink / raw)
  To: John Wiegley, Eli Zaretskii; +Cc: tino.calancha, emacs-devel

> Let me give a hypothetical example: Assume for the sake of argument that
> `count-lines' did not report characters as well, and that someone wished
> to extend the `count-lines' command so that, given a prefix arg, it would
> count characters instead of lines.
> 
> What I think should happen in this case is that a new command be created:
> count-items-in-region, which by default counts lines, but with a prefix
> argument counts characters. This leaves that command open to many new
> behaviors in future, while `count-lines' keeps doing just what it says:
> count lines.
> 
> What I would object to is adding a new argument to `count-lines', called
> `characters-p', that changes the behavior of count-lines to now count
> characters instead (again, remember this is hypothetical, I know that
> today's `count-lines' already counts characters as well).

Seems like you're confusing a few things.

Is your point about the evolution of an existing function?  You
don't want to affect the longtime behavior of an existing function?

Or is it about having a single function (`count-items-in-region'),
from the outset, that can count different things depending on the 
presence/value of an optional argument CHARACTERS-P or ITEM-TYPE?

Is it the multiple behaviors that bother you, or the fact that
some existing function is being changed/affected?

> I believe -- very strongly -- that each function should do
> one thing and one thing well,

Define "one thing".  Does `mapc' do one thing: map a function
over a list; or does it do any number of things, depending on
its function argument (and its list argument, for that matter).

Is your argument limited to behavior from &optional arguments,
or does it apply also to required args too (e.g. the FUNCTION
arg of `mapc').  Does it make a difference in your understanding
of "do one thing" whether `count-items-in-region' takes a
required CHARACTERS-P or ITEM-TYPE arg or that arg is optional?

> and this one thing should be documented well. I don't like
> magical functions with lots of alternative behaviors,

What constitutes "magical", for you?  How many alternative
behaviors is "lots", for you?

Yes, it would annoy me too if I had to try to answer such
questions. ;-)  But that's the cost of such generalizing, I
think.  Better to discuss such things in specific, concrete
cases.  That keeps arguments a bit more grounded.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 23:26                 ` Drew Adams
@ 2016-09-17 23:51                   ` John Wiegley
  2016-09-18  1:45                     ` Drew Adams
  0 siblings, 1 reply; 27+ messages in thread
From: John Wiegley @ 2016-09-17 23:51 UTC (permalink / raw)
  To: Drew Adams; +Cc: Eli Zaretskii, tino.calancha, emacs-devel

>>>>> Drew Adams <drew.adams@oracle.com> writes:

> Seems like you're confusing a few things.

I'm not quite sure why you're even asking these questions, Drew, so I think
I'll withdraw from this discussion, since nothing useful is being achieved.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Ibuffer: w and B default to buffer at current line
  2016-09-17 23:51                   ` John Wiegley
@ 2016-09-18  1:45                     ` Drew Adams
  2016-09-18  2:18                       ` John Wiegley
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2016-09-18  1:45 UTC (permalink / raw)
  To: John Wiegley; +Cc: Eli Zaretskii, tino.calancha, emacs-devel

> I'm not quite sure why you're even asking these questions, Drew, so I
> think I'll withdraw from this discussion, since nothing useful is being
> achieved.

Trying to find out what you are really saying, and how it might
apply to Emacs development.  You have a strong belief that each
function should do one thing and one thing well.  It would be
helpful, if that is to be a guideline or if we can learn from
it, to flesh out what you mean by it.

If it's clear to you and you don't care whether it's clear to
others, that's OK too.  As I said, I think that such things
are usually clearer in the context of discussing concrete
features or code.  No doubt there will be occasions to learn
what you mean for specific examples.

My guess is that what you prefer here is probably something
I would prefer too, but I'm not sure.  The devil, as well as
interesting guidelines, are likely in the details.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-18  1:45                     ` Drew Adams
@ 2016-09-18  2:18                       ` John Wiegley
  0 siblings, 0 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-18  2:18 UTC (permalink / raw)
  To: Drew Adams; +Cc: Eli Zaretskii, tino.calancha, emacs-devel

>>>>> Drew Adams <drew.adams@oracle.com> writes:

> Trying to find out what you are really saying, and how it might apply to
> Emacs development. You have a strong belief that each function should do one
> thing and one thing well. It would be helpful, if that is to be a guideline
> or if we can learn from it, to flesh out what you mean by it.

> My guess is that what you prefer here is probably something I would prefer
> too, but I'm not sure. The devil, as well as interesting guidelines, are
> likely in the details.

I had expected (naively?) that everyone would want this, since the intent is
to make code easier to learn and to maintain. It's natural to assume that a
function called "string-append" just appends strings, for example. It would be
quite surprising if it had a special behavior when the arguments are both
numbers. I know that some languages applaud this sort of hidden behavior, but
I find it confusing.

But maybe you're right, and I should just wait until there are other, more
specific contexts for me to champion this idea.

The basic philosophy behind "do one thing, do it well" is that a function
named `plus' should just add numbers; and a function called `save-file' should
just save files. If a function were named "dwim-at-point", then it could do
whatever it thinks is appropriate at point.

Since we don't have types in Emacs Lisp to enforce function semantics, we rely
on naming and documentation to figure out what things mean. I find myself
surprised that a function advertising itself as acting on "marked-files",
under some circumstances acts on unmarked files...

I mean nothing beyond this, but this can wait until it becomes clearer over
time. Thank you for your patience, and your interest.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 21:35               ` John Wiegley
  2016-09-17 23:26                 ` Drew Adams
@ 2016-09-18 14:26                 ` Eli Zaretskii
  2016-09-18 19:35                   ` John Wiegley
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-18 14:26 UTC (permalink / raw)
  To: John Wiegley; +Cc: tino.calancha, drew.adams, emacs-devel

> From: John Wiegley <jwiegley@gmail.com>
> Cc: drew.adams@oracle.com,  emacs-devel@gnu.org,  tino.calancha@gmail.com
> Date: Sat, 17 Sep 2016 14:35:57 -0700
> 
> > If DWIM is okay in the UI, then functions that behave in support of that UI
> > should also be okay.
> 
> Since this responses surprised me a bit, I'm going to assume that I've
> misunderstood somehow.
> 
> Let me give a hypothetical example: Assume for the sake of argument that
> `count-lines' did not report characters as well, and that someone wished to
> extend the `count-lines' command so that, given a prefix arg, it would count
> characters instead of lines.
> 
> What I think should happen in this case is that a new command be created:
> count-items-in-region, which by default counts lines, but with a prefix
> argument counts characters. This leaves that command open to many new
> behaviors in future, while `count-lines' keeps doing just what it says: count
> lines.

If we have 2 functions, one only for counting lines and another only
for counting characters, then how will we give a user a command that
can do both depending on the prefix argument?

In Emacs, every command is a function, so eventually, we would need to
have a single function which could do both, right?  Therefore, the
only issue here is whether that function should be called count-lines
or something else, right?

Now let's make one step forward, and imagine a command that decides
whether or not to count characters as well as lines based on some
context, whatever that may be, and imagine that such a command could
reliably make that decision.  We will then have a function with such a
DWIMish modus operandi, which is totally okay, and is actually the
_only_ way of having such commands in Emacs -- by writing a function
which implements that DWIM.

IOW, as long as having DWIM in a command is okay, it should also be
okay in a function, because in Emacs every command is a function.

> What I would object to is adding a new argument to `count-lines', called
> `characters-p', that changes the behavior of count-lines to now count
> characters instead (again, remember this is hypothetical, I know that today's
> `count-lines' already counts characters as well).

Then how would you implement a command which could do both, under some
circumstances?

> Just because I want DWIM from my interface, doesn't mean I need to implement
> it as DWIM in my functions.

But there are no user interfaces in Emacs except functions.  And if
you are saying that DWIM can only be present in top-level functions
that have an interactive spec, then (a) I think this is too
restrictive and will cause us to put too much code on the top level,
and (b) we cannot prevent Lisp programs from invoking commands as
functions.

> I believe -- very strongly -- that each function should do one thing
> and one thing well, and this one thing should be documented well. I
> don't like magical functions with lots of alternative behaviors,
> unless it is a command created for the purpose of magically
> dispatching to other functions based on its context of use. Such
> magical interface functions are quite alright in my book; but
> complexifying the behavior of core functions is not.

This all boils down to the definition of "core functions", I guess.
If you want to apply this rule only to core functions, you had better
came up with a very good and precise definition.  I predict that this
would be hard to do, and we will have many incidents of Lisp programs
that try to call non-core DWIM functions and requests to remove a
function from the core to make it more DWIMish.  IOW, I believe this
goal is not reachable in Emacs, not in practice anyway.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 19:33                   ` Drew Adams
@ 2016-09-18 14:29                     ` Eli Zaretskii
  0 siblings, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-18 14:29 UTC (permalink / raw)
  To: Drew Adams; +Cc: jwiegley, emacs-devel, tino.calancha

> Date: Sat, 17 Sep 2016 12:33:27 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: jwiegley@gmail.com, tino.calancha@gmail.com, emacs-devel@gnu.org
> 
> > > Yes, of course.  But as I pointed out, the case at hand has
> > > nothing to do with DWIM.
> > 
> > I disagree.
> 
> And your reason is?
> 
> A user _explicitly specifies_ which to use, of the behaviors
> the command supports.  How is that guess-what-I-mean-and-do-it?

See what John wrote.  The issue at hand is broader than what you
interpreted it to be.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* naming functions [was: Ibuffer: w and B default to buffer at current line]
       [not found]                     ` <<83poo1i8nf.fsf@gnu.org>
@ 2016-09-18 17:55                       ` Drew Adams
  2016-09-18 19:23                         ` John Wiegley
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2016-09-18 17:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jwiegley, emacs-devel, tino.calancha

> > > > Yes, of course.  But as I pointed out, the case at 
> > > > hand has nothing to do with DWIM.
> > >
> > > I disagree.
> >
> > And your reason is?  A user _explicitly specifies_
> > which to use, of the behaviors the command supports.
> > How is that guess-what-I-mean-and-do-it?
> 
> See what John wrote.  The issue at hand is broader than what 
> you interpreted it to be.

Please point to something that John said that you think speaks
for you here and answers the question: What is your reason for
thinking that simply having a prefix arg choose an alternative
behavior is DWIM?

> > Is `forward-char` a DWIM command, simply because it can
> > move different distances depending on a prefix arg?

Your answer to that is yes?  If so, why?

----

Wrt command (and other function) names (John's point):
Name choices are limited.  Doc strings are there to
clarify things.

John:

Is `forward-char' a bad name, because with a negative
prefix arg it moves backward?  Is it a bad name because
it is singular and a prefix arg > 1 moves further than
one position?

Should the name have been `move-char' or `move-chars'?
If so, then what about binding forward and backward
default behaviors to different keys, `C-f' and `C-b'?

I think (hope) you get the point.  A function name only
goes so far toward indicating what the function does.
If a prefix arg to a command chooses alternative behavior,
then it would often be cumbersome and _less_ clear to
users, to use a name that tries to summarize all of the behaviors.

Finally, adding `dwim' to a command name is really just
punting - pretty much all of the time.  By definition, it
says almost nothing about what the command does.  In the
worst case it says, loud and clear, "I can't give a good
name to this command.  I can barely describe it in the
doc string."

Using `dwim' in a command name tells the user only this:

  This command has multiple behaviors, some of which you
  might be able to control, and some of which might be
  determined otherwise by the use context.  See the doc
  string for what this command really does.

Consider these vanilla Emacs commands, which comment and
uncomment the region: `comment-region', `comment-dwim',
`comment-or-uncomment-region', `comment-line'.

They all comment & uncomment the region, but in different
ways.  None of those names is great, and none of those
commands is more or less DWIMish than the others.

OK, such naming is historical.  `comment-region' was
first.  `comment-or-uncomment-region' was added to
provide a slightly different behavior.  Its name more
clearly points out that it can be used to UNcomment.

(It is unfortunate, IMO, that more users do not think
to use `comment-region', including for UNcommenting,
because it is a great command.  Its uncommenting
behavior is superior to that of `comment-dwim',
`comment-line', and `comment-or-uncomment-region'.
But yes, its name does not advertise the fact that
it can uncomment.)

Presumably, John, between the two names `comment-region'
and `comment-or-uncomment-region' you would favor the
latter.  I would too, and it is much better than
`comment-dwim', which says only, "Sorry - see the doc
string to find out what this command does."

But consider that there are _many possible_ commands
that comment or uncomment region text.  The above are
all in vanilla Emacs, and I and others have come up
with still other such commands.  (I use my command
`comment-region-lines', which is like `comment-region',
except that it (un)comments whole lines that are
(at least partly) included in the region.)

Emacs is made to end up with multiple such commands.
They cannot EACH be called `comment-or-uncomment-region'
or even `comment-dwim'.

And to name each of them in a way that would describe
its behavior more clearly and _distinguish it from the
others_ would result in long and incomprehensible names.

And then, when another such command is defined (in Emacs
proper or in a 3rd-party library), some or all of them
might (logically) need to be REnamed, to properly
distinguish all of the meanings.

This might be seen as less of a problem if you ignore
the wider Emacs world outside core Emacs development.
But it is nevertheless a difficulty _inherent in the
nature_ of something like Emacs.

Names only go so far, toward conveying behavior/meaning.
And naming is historical.  And labels such as "dwim' do
NOT help to convey the specific meaning of a command.
They just punt to the doc string.

So while this general principle is a good one:

  A function's name should describe its behavior,
  and none of the behavior should be hidden behind a
  misleading name.

In practice, in a context like Emacs, things are more
complex.

This is particularly the case when it comes to commands.
And particularly when there are two commands, bound to
keys, that are more or less symmetrical - such as
`forward-char' and `backward-char'.  Each is, in fact,
a command that moves point one or more chars backward
or forward.  Neither moves only forward or only backward.
But we want 2 different commands, for 2 different keys.

In Emacs, the command naming policy has generally been
this: Name a command for its _default_ behavior, and
describe all of the alternative behaviors in the doc
string.  That works pretty well, in general.

Yes, the default behavior does NOT tell the whole
story - by definition.  But it is often, even
generally, the case that a command name _cannot_
usefully tell the whole story.  (That does not imply
that the command behavior is ill-conceived.)

In particular, putting `dwim' in a command name is a
poor cop-out.  The default behavior should generally
be used for the command name, even for a DWIM command.

(On the other hand, if it is simple to describe the
various behaviors in the name, then that can sometimes
be better.  `comment-or-uncomment-region' is better
than just `comment-region').

Some might think that putting `dwim' in a command
name is important for advertising the fact that there
_are_ multiple behaviors.  I don't buy that argument.
It is just another Look-ma-no-hands!, Tu-m'as-vu !
exclamation, akin to NEW!!!  IMPROVED!!!  SUPER!!!!

There might be a temporary effect, for a new command
named `*-dwim-*', that it attracts attention and gives
the (mistaken) impression that other commands do _not_
have multiple behaviors or do _not_ have behavior that
depends on the context.  Over time, such a name is
seen for what it is: "dwim" in the name is just noise.

We should avoid lazily naming commands with `dwim'.
It's the equivalent of the Marketing-Speak "smart":

  smart car, smart phone, smart train, smart tech,
  smart goal, smart tuition, smart exchange, smart
  growth, smart manufacturing, smart health, smart
  grid, smart cloud, smart design, smart card, smart
  package, smart kit, smart keyboard, smart material,
  smart TV,...

Nothing is duller.  Nothing says less that is helpful.
DWIM, like WYSIWYG, is a clever acronym.  But it
doesn't say much, if anything.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: naming functions [was: Ibuffer: w and B default to buffer at current line]
  2016-09-18 17:55                       ` naming functions [was: Ibuffer: w and B default to buffer at current line] Drew Adams
@ 2016-09-18 19:23                         ` John Wiegley
  2016-09-18 23:24                           ` Drew Adams
  2016-09-19 16:35                           ` Eli Zaretskii
  0 siblings, 2 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-18 19:23 UTC (permalink / raw)
  To: Drew Adams; +Cc: Eli Zaretskii, emacs-devel, tino.calancha

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

>>>>> Drew Adams <drew.adams@oracle.com> writes:

> Is `forward-char' a bad name, because with a negative prefix arg it moves
> backward? Is it a bad name because it is singular and a prefix arg > 1 moves
> further than one position?

> Should the name have been `move-char' or `move-chars'? If so, then what
> about binding forward and backward default behaviors to different keys,
> `C-f' and `C-b'?

> I think (hope) you get the point. A function name only goes so far toward
> indicating what the function does. If a prefix arg to a command chooses
> alternative behavior, then it would often be cumbersome and _less_ clear to
> users, to use a name that tries to summarize all of the behaviors.

The only point I'm seeing here is that we've been imprecise with our function
names in the past. That is not a valid argument for how we should assess
patches in the future.

Yes, `forward-char' is misleading. `move-point' might have been a better
choice. But there are many ships like this that left the port long ago. I'm
not even suggesting we retroactively fix any of them. What I will do, however,
is reject the belief that because it happened in the past, this makes it OK to
extend existing functions like this for the sake of expediency.

So unless you're actually arguing for it to be OK to extend functions beyond
their stated semantics, I hope we can agree on the essential point here: don't
pollute the behavior of functions because it's easy to do so. With just a bit
of extra work, we can avoid unnecessarily increasing our technical debt.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-18 14:26                 ` Eli Zaretskii
@ 2016-09-18 19:35                   ` John Wiegley
  0 siblings, 0 replies; 27+ messages in thread
From: John Wiegley @ 2016-09-18 19:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tino.calancha, drew.adams, emacs-devel

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

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> In Emacs, every command is a function, so eventually, we would need to have
> a single function which could do both, right? Therefore, the only issue here
> is whether that function should be called count-lines or something else,
> right?

OK, I hear you now, Eli, thanks for clarifying. You are perfectly right, every
command is also a function. In the example of counting lines and characters,
my preference would have been for there to be three functions:

    count-lines
    count-characters
    count

Where the third is the DWIM-y interactive command. This keeps the behavior of
count-lines and count-characters very clear, while the behavior of `count' is
not very clear and justifiably needs looking up in the manual to be sure what
it does.

Now, I'm not saying every package HAS to be architected this way. What I
dislike is something fairly specific: "shoe-horning" in the ability to count
characters into count-lines, simply because that's easier to do than turning
one function into three. That makes count-lines a lot more complicated, for
what seems to me to be no good reason at all.

I *think* that your point about "every command is also a function" is just a
bit orthogonal to whether we should be multiplying the semantics of our
functions -- be they commands or not. Clearly "string-append" should not start
logging to disk, or "current-time" suddenly gain the ability to compute the
volumes of spheres based on special arguments.

If your point is that this is vague, and can only be decided on a case-by-case
basis, then yes, you've convinced me of that. I'll speak up whenever I see it
happen, and we can discuss again in the context of particular issues.

For the patch at hand, if no one else has a problem with mirroring the
structure of dired in ibuffer, I'll go with the majority opinion. I guess
sometimes it can even be easier to maintain the same structure in two places,
than to have the architectures diverge because of differing principles.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: naming functions  [was: Ibuffer: w and B default to buffer at current line]
  2016-09-18 19:23                         ` John Wiegley
@ 2016-09-18 23:24                           ` Drew Adams
  2016-09-19 16:35                           ` Eli Zaretskii
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2016-09-18 23:24 UTC (permalink / raw)
  To: John Wiegley; +Cc: Eli Zaretskii, emacs-devel, tino.calancha

> > Is `forward-char' a bad name, because with a negative
> > prefix arg it moves backward? Is it a bad name because
> > it is singular and a prefix arg > 1 moves further than
> > one position?
> 
> > Should the name have been `move-char' or `move-chars'?
> > If so, then what about binding forward and backward
> > default behaviors to different keys, `C-f' and `C-b'?
> 
> > I think (hope) you get the point. A function name only
> > goes so far toward indicating what the function does.
> > If a prefix arg to a command chooses alternative behavior,
> > then it would often be cumbersome and _less_ clear to
> > users, to use a name that tries to summarize all of the
> > behaviors.
> 
> The only point I'm seeing here is that we've been imprecise
> with our function names in the past.

No, that is not the point.  I tried to make clear that in
this example `forward-char' is a good name, not a bad name.

It is _not_ a bad name just because it is singular and a
prefix arg > 1 moves further than one position.  It is _not_
a bad name just because a negative prefix arg reverses its
direction from forward to backward.

It is a _good_ name because it describes the _default_
behavior well: move forward one character.  And the doc
string, not the function name, has the job of describing
the full behavior.

It is not often that a command that has multiple possible
behaviors can reasonably have all of those behaviors
reflected well in the command name.  When it can, great.
But what if it cannot?

Even the simple command `forward-char' (few are simpler)
has multiple behaviors.  Some might even call it "dwim"
because of this (I wouldn't).

Even this simple command raises interesting questions that
speak to points that you raised about the behavior and name
of a function (command in this case).  It doesn't get much
simpler than this, and yet the issues for design policy
discussed here so far are all present in this case.

> That is not a valid argument for how we should assess
> patches in the future.

No, of course not.  But no one suggested it is.  No one
has made the argument you seem to keep opposing, that
_just because_ something misguided was done in the past,
it should be done again.

> Yes, `forward-char' is misleading. `move-point' might
> have been a better choice.

You see, that's exactly _not_ the point I was making.

(And not because `move-point' doesn't mention the unit of
the move: char, or buffer position (vs word, sexp, etc.).)

Please see this question, which you did not address, and
which gets to the point:

  > > If [you think that `forward-char' should have been
  > > called `move-point'], then what about binding forward
  > > and backward default behaviors to different keys,
  > > `C-f' and `C-b'?

If we want the convenience of two different key bindings,
one moving forward by default and the other backward by
default, AND if we want the convenience of binding those
keys to named commands (and not, e.g. for backward,
(lambda (n) (interactive "p") (move-point (- n)))), then
what to call those two commands?

It's a serious question.  `move-point-1' & `move-point-2'?
Nah, those aren't great.  What would you call them, if
you could start over?

And if you want the names here to try to reflect what
these two commands actually can do, summarizing all of
the behaviors, then those names will be, I think,

  > > cumbersome and _less_ clear to users

What would you propose for those two different command
names, if not `forward-char' and `backward-char'?
`move-point' won't work for both.

Would you go for something like this?

`move-forward-1-char-but-maybe-more-and-maybe-backward'
`move-backward-1-char-but-maybe-more-and-maybe-forward'

Again, nah; those aren't great.  But it's a serious
question, starting from the aim of wanting names that
don't mislead by suggesting there is only one direction
and only one distance of movement.  If the command names
are to reflect more than the default case, what should
they be in this (simple) case?

Emacs policy/convention/tradition/history/habit has
generally been to name commands after their _default_
behaviors:

* singular: `char', not `chars' - even though a prefix
  arg can result in moving a different distance
* one with `forward' in the name and one with `backward' -
  even though a negative prefix arg reverses the direction

That's a simple approach.  It's the best I've come across.
But I truly would like to hear of something better, for
Emacs.

> What I will do, however, is reject the belief that because
> it happened in the past, this makes it OK to extend
> existing functions like this for the sake of expediency.

Again, that's good, but I don't think anyone has made
such an argument.

> I hope we can agree on the essential point here: don't
> pollute the behavior of functions because it's easy to
> do so.  With just a bit of extra work, we can avoid
> unnecessarily increasing our technical debt.

AFAICT, no one has disagreed with that - but good.  Agreed.

That, however, does not address the interesting issues you
raised, about function naming and about when/whether/how a
function can have multiple, alternative behaviors.

I'm comfortable with what Emacs has generally done in the
past in this regard, but we can certainly put it in question,
if you feel strongly about it.  Maybe there is a better
approach?

What would be the effect of a one-behavior-per-function
policy and a function-name-reflects-full-behavior policy?
What would those mean, for instance, for simple commands
such as `forward-char' and `backward-char'?

Yes, I know that you acknowledged that commands, as opposed
to non-interactive functions, _can_ have multiple behaviors.
But my impression is that you still would like to see a
command name reflect the full behavior.  I would too, when
feasible, but I think that is _not_ the usual case.

As for what behaviors should be combined in the same command,
I'm happy to see that you've said essentially the same thing
I said: The behaviors need to somehow belong together - a
command should not be a hodge-podge of unrelated behaviors.

People can disagree about what specific behaviors belong
together, but hopefully we can at least agree on the general
idea of them being related in some useful way.  (There will
always be lumpers and splitters, etc.)



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: naming functions [was: Ibuffer: w and B default to buffer at current line]
  2016-09-18 19:23                         ` John Wiegley
  2016-09-18 23:24                           ` Drew Adams
@ 2016-09-19 16:35                           ` Eli Zaretskii
  1 sibling, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2016-09-19 16:35 UTC (permalink / raw)
  To: John Wiegley; +Cc: emacs-devel, drew.adams, tino.calancha

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  tino.calancha@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 18 Sep 2016 12:23:33 -0700
> 
> I *think* that your point about "every command is also a function" is just a
> bit orthogonal to whether we should be multiplying the semantics of our
> functions -- be they commands or not. Clearly "string-append" should not start
> logging to disk, or "current-time" suddenly gain the ability to compute the
> volumes of spheres based on special arguments.
> 
> If your point is that this is vague, and can only be decided on a case-by-case
> basis, then yes, you've convinced me of that. I'll speak up whenever I see it
> happen, and we can discuss again in the context of particular issues.

Yes, my point is that where exactly to draw the line in the sand is a
judgment call in each particular case.  Some of the cases are crystal
clear either way, but that's rare.  Plus, we've been lumping together
similar or related behaviors in a single function since day one; in
particular every forward-SOMETHING function will do the equivalent of
backward-SOMETHING when invoked with a negative argument; that's a
very frequent paradigm in Emacs.  And there are others.  It's
more-or-less the distinct flavor of our programming, "the Emacsy way"
of doing things, if you want.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-17 18:22   ` Ibuffer: w and B default to buffer at current line Tino Calancha
@ 2016-09-26 12:08     ` Tino Calancha
  2016-10-03 12:28       ` Tino Calancha
  0 siblings, 1 reply; 27+ messages in thread
From: Tino Calancha @ 2016-09-26 12:08 UTC (permalink / raw)
  To: Emacs developers
  Cc: John Wiegley, Eli Zaretskii, Emacs developers, drew.adams,
	tino.calancha



On Sun, 18 Sep 2016, Tino Calancha wrote:

> I have prepared to patches `D' and `W': the former follows DWIM, the later
> follows your suggestion.
>
> `D':
> * 2 files changed, 65 insertions(+), 58 deletions(-)
>
> `W':
> * 2 files changed, 52 insertions(+), 46 deletions(-)
>
> I don't have any preference for one or the other.  Both provide the same
> behaviour, so i am fine to push the one that people here prefer.

This thread became very general and interesting.  I would like to
ask one particular question:
i provided 2 patches, `W' and `D'.  The first, `W', address John
complaint, while `D' follows the original proposal.
As i said, i have no preference between, i just want to provide
the missing functionality.  Both patches provide the same.
The question is, what patch should apply?
Thanks.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ibuffer: w and B default to buffer at current line
  2016-09-26 12:08     ` Tino Calancha
@ 2016-10-03 12:28       ` Tino Calancha
  0 siblings, 0 replies; 27+ messages in thread
From: Tino Calancha @ 2016-10-03 12:28 UTC (permalink / raw)
  To: Tino Calancha; +Cc: John Wiegley, Eli Zaretskii, drew.adams, Emacs developers



On Mon, 26 Sep 2016, Tino Calancha wrote:

>
>
> On Sun, 18 Sep 2016, Tino Calancha wrote:
>
>> I have prepared to patches `D' and `W': the former follows DWIM, the later
>> follows your suggestion.
>> 
>> `D':
>> * 2 files changed, 65 insertions(+), 58 deletions(-)
>> 
>> `W':
>> * 2 files changed, 52 insertions(+), 46 deletions(-)
>> 
>> I don't have any preference for one or the other.  Both provide the same
>> behaviour, so i am fine to push the one that people here prefer.
>
> This thread became very general and interesting.  I would like to
> ask one particular question:
> i provided 2 patches, `W' and `D'.  The first, `W', address John
> complaint, while `D' follows the original proposal.
> As i said, i have no preference between, i just want to provide
> the missing functionality.  Both patches provide the same.
> The question is, what patch should apply?

I don't see further comments so i have chosen the patch
`W', i.e., the one following John's way.
I have pushed it to master branch as commit a7e9d1cc



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2016-10-03 12:28 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14  5:35 Ibuffer: w and B default to buffer at current line Tino Calancha
2016-09-14  6:41 ` John Wiegley
2016-09-14  7:21   ` Tino Calancha
2016-09-14 14:08     ` Drew Adams
2016-09-15 22:05     ` John Wiegley
2016-09-16  6:40       ` Eli Zaretskii
     [not found]       ` <<83intw5our.fsf@gnu.org>
2016-09-16 14:53         ` Drew Adams
2016-09-17 16:30           ` John Wiegley
2016-09-17 17:21             ` Eli Zaretskii
2016-09-17 21:35               ` John Wiegley
2016-09-17 23:26                 ` Drew Adams
2016-09-17 23:51                   ` John Wiegley
2016-09-18  1:45                     ` Drew Adams
2016-09-18  2:18                       ` John Wiegley
2016-09-18 14:26                 ` Eli Zaretskii
2016-09-18 19:35                   ` John Wiegley
     [not found]             ` <<83zin630i9.fsf@gnu.org>
2016-09-17 18:47               ` Drew Adams
2016-09-17 19:25                 ` Eli Zaretskii
     [not found]                 ` <<83vaxuib1p.fsf@gnu.org>
2016-09-17 19:33                   ` Drew Adams
2016-09-18 14:29                     ` Eli Zaretskii
     [not found]                   ` <<d33a60f5-b8b6-4637-b3e6-ea1b09d98f85@default>
     [not found]                     ` <<83poo1i8nf.fsf@gnu.org>
2016-09-18 17:55                       ` naming functions [was: Ibuffer: w and B default to buffer at current line] Drew Adams
2016-09-18 19:23                         ` John Wiegley
2016-09-18 23:24                           ` Drew Adams
2016-09-19 16:35                           ` Eli Zaretskii
2016-09-17 18:22   ` Ibuffer: w and B default to buffer at current line Tino Calancha
2016-09-26 12:08     ` Tino Calancha
2016-10-03 12:28       ` Tino Calancha

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).