* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
@ 2009-03-08 23:06 Reiner Steib
2009-03-09 17:48 ` Dan Nicolaescu
2009-03-09 17:49 ` Dan Nicolaescu
0 siblings, 2 replies; 9+ messages in thread
From: Reiner Steib @ 2009-03-08 23:06 UTC (permalink / raw)
To: emacs-pretest-bug
Hi,
> Please describe exactly what actions triggered the bug
> and the precise symptoms of the bug:
- C-x C-f lisp/gnus/gnus.el RET
- M-x vc-annotate RET
- D
Instead of showing diff of revision at line (as in Emacs 22), I get:
,----[ *Messages* ]
| vc-annotate-show-changeset-diff-revision-at-line:
| The CVS backend does not support changeset diffs
`----
Is this change intended? I could understand that `D' should now work
on change sets, but please at least mention it in NEWS and tell the
user to try `d' instead or better just display the warning and than
call `vc-annotate-show-diff-revision-at-line' if no change set
operation is available.
- L
Instead of showing log of revision at line (as in Emacs 22), nothing
happens.
If there's no useful binding for `L', why not bind it to
`vc-annotate-show-log-revision-at-line' or at least tell the user to
use `l' (lowercase L) and also document it in NEWS.
I finally found something in NEWS (first I search for "vc-annotate"
without success):
,----[ NEWS ]
| *** In VC Annotate mode, for VC systems that support changesets, you can
| see the diff for the whole changeset (not only for the current file)
| by typing the D key. Using the "Show changeset diff of revision at
| line" menu entry does the same thing.
`----
> In GNU Emacs 23.0.91.1 (i686-pc-linux-gnu, GTK+ Version 2.12.9)
> of 2009-03-08 on primula
> Windowing system distributor `The X.Org Foundation', version 11.0.10400090
Bye, Reiner.
--
,,,
(o o)
---ooO-(_)-Ooo--- | PGP key available | http://rsteib.home.pages.de/
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-08 23:06 bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22 Reiner Steib
@ 2009-03-09 17:48 ` Dan Nicolaescu
2009-03-09 19:57 ` Reiner Steib
2009-03-09 17:49 ` Dan Nicolaescu
1 sibling, 1 reply; 9+ messages in thread
From: Dan Nicolaescu @ 2009-03-09 17:48 UTC (permalink / raw)
To: Reiner Steib; +Cc: emacs-pretest-bug, 2604
Reiner Steib <reinersteib+gmane@imap.cc> writes:
> Hi,
>
> > Please describe exactly what actions triggered the bug
> > and the precise symptoms of the bug:
>
> - C-x C-f lisp/gnus/gnus.el RET
>
> - M-x vc-annotate RET
>
> - D
>
> Instead of showing diff of revision at line (as in Emacs 22), I get:
>
> ,----[ *Messages* ]
> | vc-annotate-show-changeset-diff-revision-at-line:
> | The CVS backend does not support changeset diffs
> `----
>
> Is this change intended?
It is.
> I could understand that `D' should now work
> on change sets, but please at least mention it in NEWS and tell the
> user to try `d' instead or better just display the warning and than
> call `vc-annotate-show-diff-revision-at-line' if no change set
> operation is available.
I am not sure that is better. But if you can suggest a better wording
for the error message, go for it.
> - L
>
> Instead of showing log of revision at line (as in Emacs 22), nothing
> happens.
>
> If there's no useful binding for `L', why not bind it to
> `vc-annotate-show-log-revision-at-line' or at least tell the user to
> use `l' (lowercase L) and also document it in NEWS.
That would be a bad idea. All the bindings for vc-annotate were upper
case. And that was only because vc-annotate was derived from
`view-mode' which was binding almost all lower case keys.
With more keys to bind we can add better functionality to vc-annotate
(like the 'd' and 'D' bindings), so it would be a bad idea to
Yes, it would take a tiny bit of effort for the users to get used to the
lower case bindings, but given that they are easier to type it should
not be too bad.
I'll make a note in NEWS about the changed bindings.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-09 17:48 ` Dan Nicolaescu
@ 2009-03-09 19:57 ` Reiner Steib
2009-03-10 0:42 ` Dan Nicolaescu
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Reiner Steib @ 2009-03-09 19:57 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 2604
On Mon, Mar 09 2009, Dan Nicolaescu wrote:
> Reiner Steib <reinersteib+gmane@imap.cc> writes:
> > - D
> >
> > Instead of showing diff of revision at line (as in Emacs 22), I get:
> >
> > ,----[ *Messages* ]
> > | vc-annotate-show-changeset-diff-revision-at-line:
> > | The CVS backend does not support changeset diffs
> > `----
> >
> > Is this change intended?
>
> It is.
>
> > I could understand that `D' should now work
> > on change sets, but please at least mention it in NEWS and tell the
> > user to try `d' instead or better just display the warning and than
> > call `vc-annotate-show-diff-revision-at-line' if no change set
> > operation is available.
>
> I am not sure that is better. But if you can suggest a better wording
> for the error message, go for it.
How about this? (Same in `log-view-diff-changeset', probably.)
--8<---------------cut here---------------start------------->8---
--- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
+++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
@@ -506,7 +506,10 @@
"Visit the diff of the revision at line from its previous revision for all files in the changeset."
(interactive)
(when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
- (error "The %s backend does not support changeset diffs" vc-annotate-backend))
+ (error
+ (substitute-command-keys "The %s backend does not support changeset diffs. \
+Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
+ vc-annotate-backend))
(vc-annotate-show-diff-revision-at-line-internal nil))
(defun vc-annotate-warp-revision (revspec)
--8<---------------cut here---------------end--------------->8---
> > - L
> >
> > Instead of showing log of revision at line (as in Emacs 22), nothing
> > happens.
> >
> > If there's no useful binding for `L', why not bind it to
> > `vc-annotate-show-log-revision-at-line' or at least tell the user to
> > use `l' (lowercase L) and also document it in NEWS.
>
> That would be a bad idea.
Do you also consider the following as bad? If a future version
actually has a useful command for `L', this can be removed.
--8<---------------cut here---------------start------------->8---
--- vc-annotate.el.~1.7.~ 2009-01-09 09:52:59.000000000 +0100
+++ vc-annotate.el 2009-03-09 20:45:47.000000000 +0100
@@ -124,6 +124,12 @@
(define-key m "f" 'vc-annotate-find-revision-at-line)
(define-key m "j" 'vc-annotate-revision-at-line)
(define-key m "l" 'vc-annotate-show-log-revision-at-line)
+ (define-key m "L"
+ (lambda ()
+ (interactive)
+ (error
+ (substitute-command-keys
+ "Use \\[vc-annotate-show-log-revision-at-line] to view log."))))
(define-key m "n" 'vc-annotate-next-revision)
(define-key m "p" 'vc-annotate-prev-revision)
(define-key m "w" 'vc-annotate-working-revision)
--8<---------------cut here---------------end--------------->8---
> All the bindings for vc-annotate were upper case. And that was only
> because vc-annotate was derived from `view-mode' which was binding
> almost all lower case keys. With more keys to bind we can add
> better functionality to vc-annotate (like the 'd' and 'D' bindings),
> so it would be a bad idea to
I agree that the lower case binding are better, but the transition
should be made as smooth as possible.
> Yes, it would take a tiny bit of effort for the users to get used to
> the lower case bindings, but given that they are easier to type it
> should not be too bad.
My first reaction was that I really thought these feature won't work
anymore Emacs 23 because of the (partial) rewrite of Emacs' VC system.
> I'll make a note in NEWS about the changed bindings.
Thanks.
Bye, Reiner.
--
,,,
(o o)
---ooO-(_)-Ooo--- | PGP key available | http://rsteib.home.pages.de/
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-09 19:57 ` Reiner Steib
@ 2009-03-10 0:42 ` Dan Nicolaescu
2009-03-10 0:59 ` Stefan Monnier
2009-03-17 1:13 ` Dan Nicolaescu
2 siblings, 0 replies; 9+ messages in thread
From: Dan Nicolaescu @ 2009-03-10 0:42 UTC (permalink / raw)
To: 2604
Reiner Steib <reinersteib+gmane@imap.cc> writes:
> On Mon, Mar 09 2009, Dan Nicolaescu wrote:
>
> > Reiner Steib <reinersteib+gmane@imap.cc> writes:
> > > - D
> > >
> > > Instead of showing diff of revision at line (as in Emacs 22), I get:
> > >
> > > ,----[ *Messages* ]
> > > | vc-annotate-show-changeset-diff-revision-at-line:
> > > | The CVS backend does not support changeset diffs
> > > `----
> > >
> > > Is this change intended?
> >
> > It is.
> >
> > > I could understand that `D' should now work
> > > on change sets, but please at least mention it in NEWS and tell the
> > > user to try `d' instead or better just display the warning and than
> > > call `vc-annotate-show-diff-revision-at-line' if no change set
> > > operation is available.
> >
> > I am not sure that is better. But if you can suggest a better wording
> > for the error message, go for it.
>
> How about this? (Same in `log-view-diff-changeset', probably.)
>
> --- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
> +++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
> @@ -506,7 +506,10 @@
> "Visit the diff of the revision at line from its previous revision for all files in the changeset."
> (interactive)
> (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
> - (error "The %s backend does not support changeset diffs" vc-annotate-backend))
> + (error
> + (substitute-command-keys "The %s backend does not support changeset diffs. \
> +Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
> + vc-annotate-backend))
> (vc-annotate-show-diff-revision-at-line-internal nil))
>
> (defun vc-annotate-warp-revision (revspec)
>
> > > - L
> > >
> > > Instead of showing log of revision at line (as in Emacs 22), nothing
> > > happens.
> > >
> > > If there's no useful binding for `L', why not bind it to
> > > `vc-annotate-show-log-revision-at-line' or at least tell the user to
> > > use `l' (lowercase L) and also document it in NEWS.
> >
> > That would be a bad idea.
>
> Do you also consider the following as bad?
Yup. There are other keys too. So the choice is not just do `L', it's
do all or none. I think none is preferable in order to avoid
complications and future burden.
> > Yes, it would take a tiny bit of effort for the users to get used to
> > the lower case bindings, but given that they are easier to type it
> > should not be too bad.
>
> My first reaction was that I really thought these feature won't work
> anymore Emacs 23 because of the (partial) rewrite of Emacs' VC system.
Users can look at the menus, and see the functions are still there. For
experienced users that don't use menus, they can figure out in a number
of ways what the key bindings are.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-09 19:57 ` Reiner Steib
2009-03-10 0:42 ` Dan Nicolaescu
@ 2009-03-10 0:59 ` Stefan Monnier
2009-03-10 21:30 ` Dan Nicolaescu
2009-03-17 1:13 ` Dan Nicolaescu
2 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2009-03-10 0:59 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 2604
> --8<---------------cut here---------------start------------->8---
> --- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
> +++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
> @@ -506,7 +506,10 @@
> "Visit the diff of the revision at line from its previous revision for all files in the changeset."
> (interactive)
> (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
> - (error "The %s backend does not support changeset diffs" vc-annotate-backend))
> + (error
> + (substitute-command-keys "The %s backend does not support changeset diffs. \
> +Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
> + vc-annotate-backend))
> (vc-annotate-show-diff-revision-at-line-internal nil))
We could also fallback to the single-file diff code when the fileset is
a singleton.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-10 0:59 ` Stefan Monnier
@ 2009-03-10 21:30 ` Dan Nicolaescu
2009-03-11 2:06 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Dan Nicolaescu @ 2009-03-10 21:30 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 2604
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > --8<---------------cut here---------------start------------->8---
> > --- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
> > +++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
> > @@ -506,7 +506,10 @@
> > "Visit the diff of the revision at line from its previous revision for all files in the changeset."
> > (interactive)
> > (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
> > - (error "The %s backend does not support changeset diffs" vc-annotate-backend))
> > + (error
> > + (substitute-command-keys "The %s backend does not support changeset diffs. \
> > +Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
> > + vc-annotate-backend))
> > (vc-annotate-show-diff-revision-at-line-internal nil))
>
> We could also fallback to the single-file diff code when the fileset is
> a singleton.
This is vc-annotate, the fileset (if you are referring to the vc.el
notion of fileset) is always singleton.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-10 21:30 ` Dan Nicolaescu
@ 2009-03-11 2:06 ` Stefan Monnier
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Monnier @ 2009-03-11 2:06 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 2604
>> > --8<---------------cut here---------------start------------->8---
>> > --- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
>> > +++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
>> > @@ -506,7 +506,10 @@
>> > "Visit the diff of the revision at line from its previous revision for all files in the changeset."
>> > (interactive)
>> > (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
>> > - (error "The %s backend does not support changeset diffs" vc-annotate-backend))
>> > + (error
>> > + (substitute-command-keys "The %s backend does not support changeset diffs. \
>> > +Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
>> > + vc-annotate-backend))
>> > (vc-annotate-show-diff-revision-at-line-internal nil))
>>
>> We could also fallback to the single-file diff code when the fileset is
>> a singleton.
> This is vc-annotate, the fileset (if you are referring to the vc.el
> notion of fileset) is always singleton.
I meant changeset. But, yes, I now realize that we don't know the size
of the changeset, so we can't do any better than what we do now.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-09 19:57 ` Reiner Steib
2009-03-10 0:42 ` Dan Nicolaescu
2009-03-10 0:59 ` Stefan Monnier
@ 2009-03-17 1:13 ` Dan Nicolaescu
2 siblings, 0 replies; 9+ messages in thread
From: Dan Nicolaescu @ 2009-03-17 1:13 UTC (permalink / raw)
To: 2604
Reiner Steib <reinersteib+gmane@imap.cc> writes:
> On Mon, Mar 09 2009, Dan Nicolaescu wrote:
>
> > Reiner Steib <reinersteib+gmane@imap.cc> writes:
> > > - D
> > >
> > > Instead of showing diff of revision at line (as in Emacs 22), I get:
> > >
> > > ,----[ *Messages* ]
> > > | vc-annotate-show-changeset-diff-revision-at-line:
> > > | The CVS backend does not support changeset diffs
> > > `----
> > >
> > > Is this change intended?
> >
> > It is.
> >
> > > I could understand that `D' should now work
> > > on change sets, but please at least mention it in NEWS and tell the
> > > user to try `d' instead or better just display the warning and than
> > > call `vc-annotate-show-diff-revision-at-line' if no change set
> > > operation is available.
> >
> > I am not sure that is better. But if you can suggest a better wording
> > for the error message, go for it.
>
> How about this? (Same in `log-view-diff-changeset', probably.)
>
> --- vc-annotate.el 09 Jan 2009 09:52:59 +0100 1.7
> +++ vc-annotate.el 09 Mar 2009 20:35:28 +0100
> @@ -506,7 +506,10 @@
> "Visit the diff of the revision at line from its previous revision for all files in the changeset."
> (interactive)
> (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
> - (error "The %s backend does not support changeset diffs" vc-annotate-backend))
> + (error
> + (substitute-command-keys "The %s backend does not support changeset diffs. \
> +Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
> + vc-annotate-backend))
> (vc-annotate-show-diff-revision-at-line-internal nil))
>
> (defun vc-annotate-warp-revision (revspec)
I am not too convinced that this is better, but if you think it's
better, then feel free to go ahead and change it.
I added a note in NEWS about the key bindings.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22
2009-03-08 23:06 bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22 Reiner Steib
2009-03-09 17:48 ` Dan Nicolaescu
@ 2009-03-09 17:49 ` Dan Nicolaescu
1 sibling, 0 replies; 9+ messages in thread
From: Dan Nicolaescu @ 2009-03-09 17:49 UTC (permalink / raw)
To: Reiner Steib; +Cc: 2604
Reiner Steib <reinersteib+gmane@imap.cc> writes:
> Hi,
>
> > Please describe exactly what actions triggered the bug
> > and the precise symptoms of the bug:
>
> - C-x C-f lisp/gnus/gnus.el RET
>
> - M-x vc-annotate RET
>
> - D
>
> Instead of showing diff of revision at line (as in Emacs 22), I get:
>
> ,----[ *Messages* ]
> | vc-annotate-show-changeset-diff-revision-at-line:
> | The CVS backend does not support changeset diffs
> `----
>
> Is this change intended?
It is.
> I could understand that `D' should now work
> on change sets, but please at least mention it in NEWS and tell the
> user to try `d' instead or better just display the warning and than
> call `vc-annotate-show-diff-revision-at-line' if no change set
> operation is available.
I am not sure that is better. But if you can suggest a better wording
for the error message, go for it.
> - L
>
> Instead of showing log of revision at line (as in Emacs 22), nothing
> happens.
>
> If there's no useful binding for `L', why not bind it to
> `vc-annotate-show-log-revision-at-line' or at least tell the user to
> use `l' (lowercase L) and also document it in NEWS.
That would be a bad idea. All the bindings for vc-annotate were upper
case. And that was only because vc-annotate was derived from
`view-mode' which was binding almost all lower case keys.
With more keys to bind we can add better functionality to vc-annotate
(like the 'd' and 'D' bindings), so it would be a bad idea to
Yes, it would take a tiny bit of effort for the users to get used to the
lower case bindings, but given that they are easier to type it should
not be too bad.
I'll make a note in NEWS about the changed bindings.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-03-17 1:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-08 23:06 bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22 Reiner Steib
2009-03-09 17:48 ` Dan Nicolaescu
2009-03-09 19:57 ` Reiner Steib
2009-03-10 0:42 ` Dan Nicolaescu
2009-03-10 0:59 ` Stefan Monnier
2009-03-10 21:30 ` Dan Nicolaescu
2009-03-11 2:06 ` Stefan Monnier
2009-03-17 1:13 ` Dan Nicolaescu
2009-03-09 17:49 ` Dan Nicolaescu
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).