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