unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
@ 2013-10-29  3:32 Leo Liu
  2013-10-29 14:20 ` Drew Adams
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Leo Liu @ 2013-10-29  3:32 UTC (permalink / raw)
  To: 15746; +Cc: Karl Fogel

Occationally I have overwritten bookmarks with regrets. So maybe
something along the following lines is needed.

=== modified file 'lisp/bookmark.el'
--- lisp/bookmark.el	2013-09-11 03:31:56 +0000
+++ lisp/bookmark.el	2013-10-29 03:27:15 +0000
@@ -811,6 +811,12 @@
                      bookmark-minibuffer-read-name-map
                      nil nil defaults))))
            (and (string-equal str "") (setq str default))
+           (when (and (not no-overwrite)
+                      (bookmark-get-bookmark str)
+                      (called-interactively-p 'interactive)
+                      (not (yes-or-no-p
+                            (format "Bookmark `%s' exists; overwrite? " str))))
+             (user-error "Aborted"))
            (bookmark-store str (cdr record) no-overwrite)
 
            ;; Ask for an annotation buffer for this bookmark






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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29  3:32 bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite Leo Liu
@ 2013-10-29 14:20 ` Drew Adams
  2013-10-29 18:24 ` Karl Fogel
  2015-11-08 19:27 ` bug#15746: Fix committed to master Karl Fogel
  2 siblings, 0 replies; 19+ messages in thread
From: Drew Adams @ 2013-10-29 14:20 UTC (permalink / raw)
  To: Leo Liu, 15746; +Cc: Karl Fogel

> Occationally I have overwritten bookmarks with regrets. So maybe
> something along the following lines is needed.

It is not needed, IMO.

The point of `bookmark-set' is to set a bookmark, just like the
point of `setq' (unlike `defvar') is to set a variable value,
whether or not the variable already has a value.

As far as I am concerned, it would be OK to add a separate new
command for the behavior you want (without binding it to
`C-x r m').

Or to add a user option that `bookmark-set' tests, to optionally
(not by default) do what you want.  But the default behavior
of `bookmark-set' should not be changed this way, IMO.

(FWIW, even if you do do the wrong thing occasionally, you need
not necessarily regret it.  Your updated bookmark is not saved 
immediately, depending on your value of `bookmark-save-flag'.
If you are worried about your occasional `bookmark-set' mistakes, consider setting `bookmark-save-flag' to `nil'.  Of course, you
might also want to then check such a bookmark before explicitly
using `bookmark-save'.)


[(OT) Replying to a yes-or-no question is not a *user*
error, even if the misnamed `user-error' is appropriate here
(which it is).  Just one more indication why "user-error" is
the wrong name for its intended behavior (= any error other
than a coding error).]





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29  3:32 bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite Leo Liu
  2013-10-29 14:20 ` Drew Adams
@ 2013-10-29 18:24 ` Karl Fogel
  2013-10-29 20:09   ` Drew Adams
                     ` (2 more replies)
  2015-11-08 19:27 ` bug#15746: Fix committed to master Karl Fogel
  2 siblings, 3 replies; 19+ messages in thread
From: Karl Fogel @ 2013-10-29 18:24 UTC (permalink / raw)
  To: Leo Liu; +Cc: 15746

Leo Liu <sdl.web@gmail.com> writes:
>Occationally I have overwritten bookmarks with regrets. So maybe
>something along the following lines is needed.
>
>=== modified file 'lisp/bookmark.el'
>--- lisp/bookmark.el	2013-09-11 03:31:56 +0000
>+++ lisp/bookmark.el	2013-10-29 03:27:15 +0000
>@@ -811,6 +811,12 @@
>                      bookmark-minibuffer-read-name-map
>                      nil nil defaults))))
>            (and (string-equal str "") (setq str default))
>+           (when (and (not no-overwrite)
>+                      (bookmark-get-bookmark str)
>+                      (called-interactively-p 'interactive)
>+                      (not (yes-or-no-p
>+                            (format "Bookmark `%s' exists; overwrite? " str))))
>+             (user-error "Aborted"))
>            (bookmark-store str (cdr record) no-overwrite)
> 
>            ;; Ask for an annotation buffer for this bookmark

This is interesting.  I saw Drew's followup; there are good arguments on
both sides, but on balance I think Leo's general idea is right.

I think most users would expect that that *interactively* setting a
bookmark would confirm when overriding a previous bookmark of the same
name, instead of just silently overwriting it.

Drew might be right that `bookmark-set' should not include this
functionality itself, but then there should be a wrapper function, and
every interactive key (C-x r m) currently default bound to
`bookmark-set' should be instead set to that wrapper function, then.
IOW, that question is just a matter of internal code orgainzation, not
of user-visible functionality.

(I'm tempted to just build the check directly into `bookmark-set' as Leo
does, though, because people already have custom bindings for that, and
anyway, testing `call-interactively' is enough -- it leaves
`bookmark-set's programmatic functionality unchanged.)

Leo, there would need to be a patch to the doc string too, but I can
write that.  I would probably also change the `user-error' behavior
along the lines Drew suggested.

First, I'd like to know if anyone else has thoughts on the overall
behavior...?

-Karl





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 18:24 ` Karl Fogel
@ 2013-10-29 20:09   ` Drew Adams
  2013-10-29 20:51     ` Karl Fogel
  2013-10-30  2:11     ` Stefan Monnier
  2013-10-30  1:28   ` Leo Liu
  2013-10-30  2:26   ` Drew Adams
  2 siblings, 2 replies; 19+ messages in thread
From: Drew Adams @ 2013-10-29 20:09 UTC (permalink / raw)
  To: Karl Fogel, Leo Liu; +Cc: 15746

> I think most users would expect that that *interactively* setting a
> bookmark would confirm when overriding a previous bookmark of the
> same name, instead of just silently overwriting it.

There are plenty of use cases where you want to silently *update* an
existing bookmark, e.g., update its location.

That's the point of `bookmark-set': it both creates and updates.

> Drew might be right that `bookmark-set' should not include this
> functionality itself, but then there should be a wrapper function,
> and every interactive key (C-x r m) currently default bound to
> `bookmark-set' should be instead set to that wrapper function, then.
> IOW, that question is just a matter of internal code orgainzation,
> not of user-visible functionality.

That effectively just *replaces* interactive use of `bookmark-set'
with the proposed alternative behavior.  That is exactly what I
object to.

I do not object to adding a different command, and either not binding
it by default (users can choose to do that) or binding it to a
different key.  That lets users choose the behavior they want, and
if they for some reason want there to be only one of the behaviors
then they can easily bind both keys to the same command or unbind a
key.

And I do not object to adding a user option that thus conditionally
changes the behavior of `bookmark-set'.

The first approach of these two alternatives gives the most
flexibility.  With it, they need not choose the behavior once and for
all but can instead just choose which behavior they want to invoke
in any given context.

My point is that it is a mistake to think there is only one interactive
use of `bookmark-set' and that for that one use users would want to be
queried wrt overwriting.

There are lots of kinds of bookmarks, and lots of different uses of
bookmarks.  And some of those call precisely for silently updating an
existing bookmark.





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 20:09   ` Drew Adams
@ 2013-10-29 20:51     ` Karl Fogel
  2013-10-29 22:16       ` Drew Adams
  2013-10-30  2:11     ` Stefan Monnier
  1 sibling, 1 reply; 19+ messages in thread
From: Karl Fogel @ 2013-10-29 20:51 UTC (permalink / raw)
  To: Drew Adams; +Cc: 15746, Leo Liu

Drew Adams <drew.adams@oracle.com> writes:
>> I think most users would expect that that *interactively* setting a
>> bookmark would confirm when overriding a previous bookmark of the
>> same name, instead of just silently overwriting it.
>
>There are plenty of use cases where you want to silently *update* an
>existing bookmark, e.g., update its location.

There are definitely cases where one wants to update; that one wants to
do so *silently* is a more subjective assertion :-).

>That's the point of `bookmark-set': it both creates and updates.

The point of `bookmark-set' is whatever its doc string says.  As long as
we don't break compatibility in some silly way, we can change its
behavior.

I realize I'm driving the point home a bit hard here, but this part of
your argument sounds to me a bit like "bookmark-set shouldn't do this
because this isn't the sort of thing it should do".

>> Drew might be right that `bookmark-set' should not include this
>> functionality itself, but then there should be a wrapper function,
>> and every interactive key (C-x r m) currently default bound to
>> `bookmark-set' should be instead set to that wrapper function, then.
>> IOW, that question is just a matter of internal code orgainzation,
>> not of user-visible functionality.
>
>That effectively just *replaces* interactive use of `bookmark-set'
>with the proposed alternative behavior.  That is exactly what I
>object to.

Yes, I understand.  The discussion here is about the user-visible
default behavior, not about how we implement it.

>I do not object to adding a different command, and either not binding
>it by default (users can choose to do that) or binding it to a
>different key.  That lets users choose the behavior they want, and
>if they for some reason want there to be only one of the behaviors
>then they can easily bind both keys to the same command or unbind a
>key.
>
>And I do not object to adding a user option that thus conditionally
>changes the behavior of `bookmark-set'.
>
>The first approach of these two alternatives gives the most
>flexibility.  With it, they need not choose the behavior once and for
>all but can instead just choose which behavior they want to invoke
>in any given context.
>
>My point is that it is a mistake to think there is only one interactive
>use of `bookmark-set' and that for that one use users would want to be
>queried wrt overwriting.

No one has proposed that there is only one valid interactive use case;
I'm not sure where you got the idea that anyone thought that.

>There are lots of kinds of bookmarks, and lots of different uses of
>bookmarks.  And some of those call precisely for silently updating an
>existing bookmark.

Yes, and some call for warning the user.  Both scenarios happen.  The
question is not whether bookmark should offer flexibility (of course it
should), but what its *default* interactive behavior should be.

Right now, I lean toward Leo's argument that bookmark has had a
less-than-ideal default up until now, and that his proposal is a better
default.  Silently overwriting an old bookmark loses information, while
confirming the overwrite does not lose information.  The only cost is
one extra interactive prompt, and *only* in the case where one is
overwriting an existing bookmark of that name.  I believe this is what
most users would expect.  (Compare: when you save a file under a new
name, if there's an existing file of that name, you are also warned.)

So I propose that that should be the default behavior (because losing
information silently is usually bad), and that there can be a variable
for you to set to get the silent-overwrite behavior that you prefer as
the default.

Equal flexibility there.  This discussion is about _defaults_.

Best,
-K





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 20:51     ` Karl Fogel
@ 2013-10-29 22:16       ` Drew Adams
  2013-10-30  4:31         ` Karl Fogel
  0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2013-10-29 22:16 UTC (permalink / raw)
  To: Karl Fogel; +Cc: 15746, Leo Liu

> >There are plenty of use cases where you want to silently *update* an
> >existing bookmark, e.g., update its location.
>
> There are definitely cases where one wants to update; that one wants
> to do so *silently* is a more subjective assertion :-).

I did not say that *all* updating use cases are silent-updating cases.
I said that there *are* cases where one wants to update silently.  And
there are.

This is not subjective to those who use this feature (for some
bookmarks, some of the time).  Consider the use of temporary bookmarks
to bounce around positions in a buffer.  ("Temporary" can be a simple
as not saving.)

Put it another way: have you ever moved a bookmark from page 39 to
page 72 when reading a book?  That's a use case for silent updating:
manually updating the location of an existing bookmark.  You do it
when you want to do it, knowing full well that the bookmark exists.

> >That's the point of `bookmark-set': it both creates and updates.
> 
> The point of `bookmark-set' is whatever its doc string says.

Being able to either create a new bookmark or update an existing
bookmark *has been* the point of `bookmark-set' - do you prefer that
wording?

> sounds to me a bit like "bookmark-set shouldn't do
> this because this isn't the sort of thing it should do".

It conflicts with *one* of the things that it should do.  And that
it has always done.  Users should not lose the convenience of being
able to silently update.  That is one of the features that
`bookmark-set' provides.

Nothing wrong with providing additional, alternative behavior, for
other use cases.  I made that clear from the outset.  But please
do not take away the behavior that makes this use case convenient.
That's the point.

> >> Drew might be right that `bookmark-set' should not include this
> >> functionality itself, but then there should be a wrapper
> >> function, and every interactive key (C-x r m) currently default
> >> bound to `bookmark-set' should be instead set to that wrapper
> >> function, then.  IOW, that question is just a matter of internal
> >> code orgainzation, not of user-visible functionality.
> >
> >That effectively just *replaces* interactive use of `bookmark-set'
> >with the proposed alternative behavior.  That is exactly what I
> >object to.
> 
> Yes, I understand.  The discussion here is about the user-visible
> default behavior, not about how we implement it.

If you provide both behaviors then it is hard to object.  That was
*not* the proposal, however.

I would also prefer not to change the *default* behavior, but for
that I would not object strongly.  Users are used to the current
behavior of `C-x r m' - it has done what it does for a long time.
My own preference would be to give the new behavior a new binding.

But as long as both are available, the binding of each is not so
important.

> >I do not object to adding a different command, and either not
> >binding it by default (users can choose to do that) or binding
> >it to a different key.  That lets users choose the behavior they
> >want, and if they for some reason want there to be only one of
> >the behaviors then they can easily bind both keys to the same
> >command or unbind a key.
> >
> >And I do not object to adding a user option that thus conditionally
> >changes the behavior of `bookmark-set'.
> >
> >The first approach of these two alternatives gives the most
> >flexibility.  With it, they need not choose the behavior once and
> >for all but can instead just choose which behavior they want to
> >invoke in any given context.
> >
> >My point is that it is a mistake to think there is only one
> >interactive use of `bookmark-set' and that for that one use users
> >would want to be queried wrt overwriting.
> 
> No one has proposed that there is only one valid interactive use
> case; I'm not sure where you got the idea that anyone thought that.

The proposal was to *change* the interactive behavior when the
bookmark already exists (and no prefix arg).  The proposal was to
*always* query the user for confirmation in this case.  That is what
I objected to.

My point was to give users a choice in that case.  Provide two
different commands. 

> >There are lots of kinds of bookmarks, and lots of different uses of
> >bookmarks.  And some of those call precisely for silently updating
> >an existing bookmark.
> 
> Yes, and some call for warning the user.  Both scenarios happen.

Precisely.  So provide for *both* uses.  No one objected to simply
*adding* the possibility of asking for confimation.  I objected to
such a new behavior *replacing* the existing behavior of updating
silently.

My point is that silent updating is not just a mistake or a bug or
always undesirable.  And I will say the same thing about querying
for confirmation.  "Both scenarios happen", as you say.  So let's
provide users an easy way to do either, au choix.  That's the point.

Doing that will be an improvement for users rather than a loss.

> The question is not whether bookmark should offer flexibility
> (of course it should), but what its *default* interactive behavior
> should be.

No, that was exactly my question/objection: the proposal removes
flexibility and choice.  Even the existence of a silent-update use
case was questioned.

Wrt the default behavior, see above.  I do not feel strongly about
that.

> Right now, I lean toward Leo's argument that bookmark has had a
> less-than-ideal default up until now, and that his proposal is a
> better default.

Leo did *not* propose a change in default behavior.  He proposed
to replace the existing behavior and instead *always* query the
user for confirmation in the case at hand.

And that replacement is what I objected to.

You seem to be turning things around, as if the proposal gave users
a choice and I were arguing against that or I were arguing only
about a different default behavior.  Look at the proposed patch,
please.

> Silently overwriting an old bookmark loses information,
> while confirming the overwrite does not lose information.  The only
> cost is one extra interactive prompt, and *only* in the case where
> one is overwriting an existing bookmark of that name.

That is not a cost that one wants to pay needlessly, in cases where
one *expects* to move the bookmark.

Imagine if someone said to you, about your moving your bookmark in
your novel: "From now on, each time you try to move your bookmark
you will need to confirm that you really want to do that, since
this is, after all, an existing bookmark."

It seems that you have not recognized the silent-update use case,
and so have not understood why the proposed change would be
annoying, hence why users need a choice here.  I hope you understand
it now.

> I believe this is what most users would expect.

Could be.  That speaks positively for the non-silent update case.
It is not an argument for denying the silent-update case.

> (Compare: when you save a file under a new name, if there's an
> existing file of that name, you are also warned.)

We can play games with such analogies, if you like.  Updating a
bookmark is more like typing text into a buffer - *saving* your
bookmarks is something different from updating them.

Yes, some buffers are read-only, and for those an error is raised
when you type.  (But you are not warned by the action of turning
off read-only - you are not asked to confirm that change.)

> So I propose that that should be the default behavior (because
> losing information silently is usually bad), and that there can
> be a variable for you to set to get the silent-overwrite behavior
> that you prefer as the default.

No.  A variable is not user friendly.  There should be two different
commands, bound to two different keys.  It is about different use
cases - for different contexts.  It is not about different users,
some of whom always want silent updating and some of whom always
want confirmation querying.

> Equal flexibility there.  This discussion is about _defaults_.

Providing a variable as the only means to silently update does
not provide equal flexibility.

There is no need for a discussion about defaults (except for which
command goes on which key), if you provide two different commands
bound to two different keys.  And that really *does* provide
"equal flexibility".





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 18:24 ` Karl Fogel
  2013-10-29 20:09   ` Drew Adams
@ 2013-10-30  1:28   ` Leo Liu
  2013-10-30  2:26   ` Drew Adams
  2 siblings, 0 replies; 19+ messages in thread
From: Leo Liu @ 2013-10-30  1:28 UTC (permalink / raw)
  To: Karl Fogel; +Cc: 15746

On 2013-10-30 02:24 +0800, Karl Fogel wrote:
> Leo, there would need to be a patch to the doc string too, but I can
> write that.  I would probably also change the `user-error' behavior
> along the lines Drew suggested.

Thank you for taking care of this.

Leo





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 20:09   ` Drew Adams
  2013-10-29 20:51     ` Karl Fogel
@ 2013-10-30  2:11     ` Stefan Monnier
  2013-10-30  2:35       ` Drew Adams
  2013-10-30  2:56       ` Leo Liu
  1 sibling, 2 replies; 19+ messages in thread
From: Stefan Monnier @ 2013-10-30  2:11 UTC (permalink / raw)
  To: Drew Adams; +Cc: Karl Fogel, 15746, Leo Liu

>> I think most users would expect that that *interactively* setting a
>> bookmark would confirm when overriding a previous bookmark of the
>> same name, instead of just silently overwriting it.
> There are plenty of use cases where you want to silently *update* an
> existing bookmark, e.g., update its location.

How 'bout the following:
- change bookmark-set to emit a clear message when it updates an
  existing message, as well as recording the previous value somewhere.
- provide a bookmark-undo command so the user can undo his
  bookmark-set when he discovers that he has just changed an existing
  bookmark by mistake.
This way, we avoid prompting Drew annoyingly when he knows full well
he's updating the bookmark.
Leo, would that be sufficient to avoid the regrets, or did you realize
too late for a bookmark-undo to be of any use?


        Stefan





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 18:24 ` Karl Fogel
  2013-10-29 20:09   ` Drew Adams
  2013-10-30  1:28   ` Leo Liu
@ 2013-10-30  2:26   ` Drew Adams
  2 siblings, 0 replies; 19+ messages in thread
From: Drew Adams @ 2013-10-30  2:26 UTC (permalink / raw)
  To: Karl Fogel, Leo Liu; +Cc: 15746

> I would probably also change the `user-error' behavior along the
> lines Drew suggested.

Uh, I did not suggest any change wrt the use of `user-error', here.
Unfortunately, it is the appropriate type of error to use in this case.

There has been abundant discussion of `user-error' in other threads.
Its meaning is not a user error.  The meaning is any error except a
coding error, i.e., except something that the code never expects to
happen.  That's OK as a category, perhaps, but it should not have the
name "user-error".  But it does, and we live with it.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11999#35
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15670#11





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  2:11     ` Stefan Monnier
@ 2013-10-30  2:35       ` Drew Adams
  2013-10-30  2:56       ` Leo Liu
  1 sibling, 0 replies; 19+ messages in thread
From: Drew Adams @ 2013-10-30  2:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Karl Fogel, 15746, Leo Liu

> How 'bout the following:
> - change bookmark-set to emit a clear message when it updates an
>   existing message, as well as recording the previous value
>   somewhere.
> - provide a bookmark-undo command so the user can undo his
>   bookmark-set when he discovers that he has just changed an
>   existing bookmark by mistake.
> This way, we avoid prompting Drew annoyingly when he knows full well
> he's updating the bookmark.
> Leo, would that be sufficient to avoid the regrets, or did you
> realize too late for a bookmark-undo to be of any use?

To quote a famous person ;-), that would really be overthinking this.

The simple solution, which I expect everyone might be able to agree
to, is to have two different commands.  And to bind them both to keys,
at least on `bookmark-map'.  Choose the default behavior you like for
the traditional key, `C-x r m'.  For the other command use, e.g.,
`C-x r M'.  Not a big deal.  Users can get either behavior at any time.

Update `bookmark-set' to check not only the case in question (same
name as existing bookmark, and no prefix arg) but also a global
variable.  Define the new command by let-binding that variable around
`call-interactively' of `bookmark-set'.





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  2:11     ` Stefan Monnier
  2013-10-30  2:35       ` Drew Adams
@ 2013-10-30  2:56       ` Leo Liu
  2013-10-30  3:14         ` Stefan Monnier
  1 sibling, 1 reply; 19+ messages in thread
From: Leo Liu @ 2013-10-30  2:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Karl Fogel, 15746

On 2013-10-30 10:11 +0800, Stefan Monnier wrote:
> How 'bout the following:
> - change bookmark-set to emit a clear message when it updates an
>   existing message, as well as recording the previous value somewhere.
> - provide a bookmark-undo command so the user can undo his
>   bookmark-set when he discovers that he has just changed an existing
>   bookmark by mistake.
> This way, we avoid prompting Drew annoyingly when he knows full well
> he's updating the bookmark.
> Leo, would that be sufficient to avoid the regrets, or did you realize
> too late for a bookmark-undo to be of any use?

Undo in this case is less intuitive than when editing text. I think the
right moment to decide overwriting is when creating the bookmark. So
maybe we could provide a variable to control this confirmation
behaviour? But I agree with Karl the default should provide
confirmation.

BTW, I use bookmark as some sort of persistent registers that I can make
use another day another place. I rsync bookmarks to a few machines so
bookmark-save-flag is 1 to make sure the db is always fully in disk.

Leo





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  2:56       ` Leo Liu
@ 2013-10-30  3:14         ` Stefan Monnier
  2013-10-30  3:36           ` Leo Liu
  2013-10-30 14:07           ` Drew Adams
  0 siblings, 2 replies; 19+ messages in thread
From: Stefan Monnier @ 2013-10-30  3:14 UTC (permalink / raw)
  To: Leo Liu; +Cc: Karl Fogel, 15746

How 'bout we use a slightly lighter weight y-or-n-p which accepts RET to
mean "yes"?
This way, Drew can hit RET RET when he knows he's overwriting an
existing bookmark.


        Stefan





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  3:14         ` Stefan Monnier
@ 2013-10-30  3:36           ` Leo Liu
  2013-10-30  3:57             ` Stefan Monnier
  2013-10-30 14:07           ` Drew Adams
  1 sibling, 1 reply; 19+ messages in thread
From: Leo Liu @ 2013-10-30  3:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Karl Fogel, 15746

On 2013-10-30 11:14 +0800, Stefan Monnier wrote:
> This way, Drew can hit RET RET when he knows he's overwriting an
> existing bookmark.

This would change a standard question into something slightly different.
Not good UI in my view.

Leo





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  3:36           ` Leo Liu
@ 2013-10-30  3:57             ` Stefan Monnier
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2013-10-30  3:57 UTC (permalink / raw)
  To: Leo Liu; +Cc: Karl Fogel, 15746

>> This way, Drew can hit RET RET when he knows he's overwriting an
>> existing bookmark.
> This would change a standard question into something slightly different.
> Not good UI in my view.

Partly agreed.  But the RET RET confirmation is already used in
C-x C-f and C-x b when visiting a new file or a new buffer, so it's not
completely new.


        Stefan





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-29 22:16       ` Drew Adams
@ 2013-10-30  4:31         ` Karl Fogel
  2013-10-30 14:07           ` Drew Adams
  0 siblings, 1 reply; 19+ messages in thread
From: Karl Fogel @ 2013-10-30  4:31 UTC (permalink / raw)
  To: Drew Adams; +Cc: 15746, Leo Liu

Drew Adams <drew.adams@oracle.com> writes:
>You seem to be turning things around, as if the proposal gave users
>a choice and I were arguing against that or I were arguing only
>about a different default behavior.  Look at the proposed patch,
>please.

Oh, sure, I saw the patch.  I saw it as a starting point; subsequent
discussion (before my mail, IIRC) turned to the question of providing a
customizeable behavior, so that both ways were available.  In any case,
I assumed that potential customization was so obvious we were clearly
just talking about the question of what the default should be.  I see
now that I should have made that assumption more explicit, however.

>It seems that you have not recognized the silent-update use case,
>and so have not understood why the proposed change would be
>annoying, hence why users need a choice here.  I hope you understand
>it now.

Actually, no -- not only do I recognize it, it is the majority use case
for me as well.  I just don't think it's good as a default, that's all.

>No.  A variable is not user friendly.  There should be two different
>commands, bound to two different keys.  It is about different use
>cases - for different contexts.  It is not about different users,
>some of whom always want silent updating and some of whom always
>want confirmation querying.

That's another solution, hmmm, but it seems to me it complexifies the
user interface a bit (it adds another binding in the keyspace, which the
user then cannot avoid encountering when looking at the available bound
commands -- whereas a variable is something they only need to deal with
if/when they go looking for it, read the documentation, etc).  So I'm
not sure which way is better; I think we might be down to the "tyranny
of small differences" at that point :-).

>Providing a variable as the only means to silently update does
>not provide equal flexibility.
>
>There is no need for a discussion about defaults (except for which
>command goes on which key), if you provide two different commands
>bound to two different keys.  And that really *does* provide
>"equal flexibility".

As far as that assertion goes, it is true, yes.  It doesn't address the
keyspace complexity issue.

I guess I'll ponder, and then commit some patch.  It need not be the
final patch, but 90% of the code is the same either way and I'd like to
get that part into the codebase so we can discuss the other 10%.

Best,
-K





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  3:14         ` Stefan Monnier
  2013-10-30  3:36           ` Leo Liu
@ 2013-10-30 14:07           ` Drew Adams
  2013-10-30 18:17             ` Stefan Monnier
  1 sibling, 1 reply; 19+ messages in thread
From: Drew Adams @ 2013-10-30 14:07 UTC (permalink / raw)
  To: Stefan Monnier, Leo Liu; +Cc: Karl Fogel, 15746

> How 'bout we use a slightly lighter weight y-or-n-p which accepts
> RET to mean "yes"?

Yes.

> This way, Drew can hit RET RET when he knows he's overwriting an
> existing bookmark.

No.  There still should be two different commands.  Updating a
bookmark should not always require interacting with a confirmation
query.

There is really no good reason for such complexity.  Make both
use cases equally easy.  Provide `C-x r m' and `C-x r M'.  Simple.





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30  4:31         ` Karl Fogel
@ 2013-10-30 14:07           ` Drew Adams
  0 siblings, 0 replies; 19+ messages in thread
From: Drew Adams @ 2013-10-30 14:07 UTC (permalink / raw)
  To: Karl Fogel; +Cc: 15746, Leo Liu

> >No.  A variable is not user friendly.  There should be two
> >different commands, bound to two different keys.  It is about
> >different use cases - for different contexts.  It is not about
> >different users, some of whom always want silent updating and some
> >of whom always want confirmation querying.
> 
> That's another solution, hmmm, but it seems to me it complexifies
> the user interface a bit 

It's a lot simpler for a user than changing a variable value for one
bookmark update (w/o confirmation) and changing it again for another
update (w/ confirmation).

> (it adds another binding in the keyspace, which the user then cannot
> avoid encountering when looking at the available bound commands

That's a good thing.  Helps users see that both possibilities exist.

> -- whereas a variable is something they only need to deal with
> if/when they go looking for it, read the documentation, etc).

Which is not a thing.  And `C-h m' and `C-h b' *are* part of the
documentation.

> So I'm not sure which way is better; I think we might be down to
> the "tyranny of small differences" at that point :-).

If you take the point of view that both are useful behaviors for a
user to have (even the same user, in different contexts), and that
it should be easy to use either of them, then having two commands
recommends itself as the way to go.  If you don't want to provide
key bindings for both commands, fine (but too bad).

> >Providing a variable as the only means to silently update does
> >not provide equal flexibility.
> >
> >There is no need for a discussion about defaults (except for which
> >command goes on which key), if you provide two different commands
> >bound to two different keys.  And that really *does* provide
> >"equal flexibility".
> 
> As far as that assertion goes, it is true, yes.  It doesn't address
> the keyspace complexity issue.

I don't see a keyspace complexity issue here.  Having both `C-x r m'
and, say, `C-x r M', which do almost the same thing, sounds quite
reasonable, to me.





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

* bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite
  2013-10-30 14:07           ` Drew Adams
@ 2013-10-30 18:17             ` Stefan Monnier
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2013-10-30 18:17 UTC (permalink / raw)
  To: Drew Adams; +Cc: Karl Fogel, 15746, Leo Liu

> No.  There still should be two different commands.  Updating a
> bookmark should not always require interacting with a confirmation
> query.

You don't need to interact: since you know you're updating a bookmark,
you just hit RET RET at the end instead of a single RET, without paying
attention to the prompt that might be displayed.


        Stefan





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

* bug#15746: Fix committed to master.
  2013-10-29  3:32 bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite Leo Liu
  2013-10-29 14:20 ` Drew Adams
  2013-10-29 18:24 ` Karl Fogel
@ 2015-11-08 19:27 ` Karl Fogel
  2 siblings, 0 replies; 19+ messages in thread
From: Karl Fogel @ 2015-11-08 19:27 UTC (permalink / raw)
  To: 15746-done

I've pushed a fix to master for this (commit 3812e17978).

Essentially it takes Drew's suggestion.  I left "C-x r m" as `bookmark-set', with the same behavior it has now, and "C-x r M" sets a bookmark but errors if it would overwrite an existing bookmark of the same name.

I didn't go with Stefan's suggestion of using RET RET instead of RET, because you can't habituate to it.  You don't necessarily know in advance whether you're setting a new bookmark or updating an existing bookmark, so you can't predict whether you will need to hit RET or RET RET.  Better to offer two different commands, and those people who want one style all the time can just use the appropriate command all the time.

Comments and further discussion welcome, of course.  Thanks, Leo, for raising this issue, and thanks Drew and Stefan for helping think things through in the bug ticket.





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

end of thread, other threads:[~2015-11-08 19:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-29  3:32 bug#15746: 24.3; [PATCH] bookmark should confirm when overwrite Leo Liu
2013-10-29 14:20 ` Drew Adams
2013-10-29 18:24 ` Karl Fogel
2013-10-29 20:09   ` Drew Adams
2013-10-29 20:51     ` Karl Fogel
2013-10-29 22:16       ` Drew Adams
2013-10-30  4:31         ` Karl Fogel
2013-10-30 14:07           ` Drew Adams
2013-10-30  2:11     ` Stefan Monnier
2013-10-30  2:35       ` Drew Adams
2013-10-30  2:56       ` Leo Liu
2013-10-30  3:14         ` Stefan Monnier
2013-10-30  3:36           ` Leo Liu
2013-10-30  3:57             ` Stefan Monnier
2013-10-30 14:07           ` Drew Adams
2013-10-30 18:17             ` Stefan Monnier
2013-10-30  1:28   ` Leo Liu
2013-10-30  2:26   ` Drew Adams
2015-11-08 19:27 ` bug#15746: Fix committed to master Karl Fogel

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).