unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
@ 2015-11-09  4:58 Zachary Kanfer
  2019-08-01 18:05 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Zachary Kanfer @ 2015-11-09  4:58 UTC (permalink / raw)
  To: 21867

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

The way that lossage now shows different commands on different lines is
really useful, but I found a way where it's not treating two different
things as different commands. To reproduce:

emacs -Q

open the scratch buffer

insert this whole s-expression:

(read-char)

Then, with point at the end of that line, type:

C-x C-e a C-h l

This evaluates the (read-char) sexp which reads the `a` typed, then
views lossage.

The last lines of lossage are:

 C-x C-e [eval-last-sexp]
 a C-h l [view-lossage]

Note that the log line calling view-lossage also includes "a", the
command read by read-char. I would expect the lossage buffer to be
something like this:

 C-x C-e [eval-last-sexp]
 a [char read by read-char]
 C-h l [view-lossage]

This does not happen with read-string; one gets logs like:

 C-x C-e [eval-last-sexp]
 p [self-insert-command]
 a [self-insert-command]
 n [self-insert-command]
 t [self-insert-command]
 s [self-insert-command]
 <return> [exit-minibuffer]
 C-h l [view-lossage]

I noted this when viewing lossage after calling org-agenda, which I
think uses read-char to get a character to dispatch on. However, I'm not
sure; the package is too complicated for me to track down how it's
reading the character. The same behavior happens when calling read-char,
so it might be the same thing.


In GNU Emacs 25.0.50.9 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2015-10-31
Repository revision: e4740877d6feeb357d7437e6025dba641800c11d
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:    Ubuntu 14.04.3 LTS

Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY GNUTLS LIBXML2 FREETYPE XFT
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Help

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
C-h j is undefined
Rebuilding agenda buffer...done
Log mode is on
Type "q" in help window to restore its previous buffer.
C-, is undefined
Press key for agenda command:
C-c l is undefined
Press key for agenda command:

C-, is undefined

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
help-fns mail-prsvr mail-utils help-mode cal-iso org-agenda org
org-macro org-footnote org-pcomplete pcomplete org-list org-faces
org-entities noutline outline easy-mmode org-version ob-emacs-lisp ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint comint
ansi-color ring ob-core ob-eval org-compat org-macs org-loaddefs
format-spec find-func cal-menu easymenu calendar cal-loaddefs edmacro
kmacro cl-loaddefs pcase cl-lib time-date mule-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 133879 6850)
 (symbols 48 25562 0)
 (miscs 40 489 115)
 (strings 32 32608 5367)
 (string-bytes 1 1089148)
 (vectors 16 16809)
 (vector-slots 8 483236 6626)
 (floats 8 171 440)
 (intervals 56 364 3)
 (buffers 976 14)
 (heap 1024 35202 1287))

[-- Attachment #2: Type: text/html, Size: 4903 bytes --]

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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2015-11-09  4:58 bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands Zachary Kanfer
@ 2019-08-01 18:05 ` Lars Ingebrigtsen
  2019-08-01 18:21   ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-01 18:05 UTC (permalink / raw)
  To: Zachary Kanfer; +Cc: 21867

Zachary Kanfer <zkanfer@gmail.com> writes:

> The way that lossage now shows different commands on different lines is
> really useful, but I found a way where it's not treating two different
> things as different commands. To reproduce:
>
> emacs -Q
>
> open the scratch buffer
>
> insert this whole s-expression:
>
> (read-char)
>
> Then, with point at the end of that line, type:
>
> C-x C-e a C-h l
>
> This evaluates the (read-char) sexp which reads the `a` typed, then
> views lossage.
>
> The last lines of lossage are:
>
>  C-x C-e [eval-last-sexp]
>  a C-h l [view-lossage]
>
> Note that the log line calling view-lossage also includes "a", the
> command read by read-char. I would expect the lossage buffer to be
> something like this:
>
>  C-x C-e [eval-last-sexp]
>  a [char read by read-char]
>  C-h l [view-lossage]
>
> This does not happen with read-string; one gets logs like:
>
>  C-x C-e [eval-last-sexp]
>  p [self-insert-command]
>  a [self-insert-command]
>  n [self-insert-command]
>  t [self-insert-command]
>  s [self-insert-command]
>  <return> [exit-minibuffer]
>  C-h l [view-lossage]

(I'm going through old bug reports that have unfortunately not gotten
any responses.)

I'm seeing the same thing in Emacs 27 -- C-x C-e on the `(read-char)'
and then a couple of <down>s:

 C-x C-e		;; eval-last-sexp
 a <down>		;; next-line
 <down>			;; next-line

This is what's returned by `recent-keys':

 24 5
 (nil . eval-last-sexp)
 97 down
 (nil . next-line)
 down
 (nil . next-line)

97 is the ?a.

The reason is that `read-char' does this:

  /* Store these characters into recent_keys, the dribble file if any,
     and the keyboard macro being defined, if any.  */
  record_char (c);
  recorded = true;
  if (! NILP (also_record))
    record_char (also_record);

But...  it's not clear what `read-char' should put into recent_keys
here, which is a not-very-clear structure.  Perhaps (nil . "char read by
read-char") after the char and then adjust `view-lossage'?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 18:05 ` Lars Ingebrigtsen
@ 2019-08-01 18:21   ` Eli Zaretskii
  2019-08-01 18:41     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-08-01 18:21 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 01 Aug 2019 20:05:29 +0200
> Cc: 21867@debbugs.gnu.org
> 
> > (read-char)
> >
> > Then, with point at the end of that line, type:
> >
> > C-x C-e a C-h l
> >
> > This evaluates the (read-char) sexp which reads the `a` typed, then
> > views lossage.
> >
> > The last lines of lossage are:
> >
> >  C-x C-e [eval-last-sexp]
> >  a C-h l [view-lossage]
> >
> > Note that the log line calling view-lossage also includes "a", the
> > command read by read-char. I would expect the lossage buffer to be
> > something like this:
> >
> >  C-x C-e [eval-last-sexp]
> >  a [char read by read-char]
> >  C-h l [view-lossage]

FWIW, I don't see why 'a' should be treated as a command here.  It
isn't.





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 18:21   ` Eli Zaretskii
@ 2019-08-01 18:41     ` Lars Ingebrigtsen
  2019-08-01 18:51       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-01 18:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21867, zkanfer

Eli Zaretskii <eliz@gnu.org> writes:

>> >  C-x C-e [eval-last-sexp]
>> >  a [char read by read-char]
>> >  C-h l [view-lossage]
>
> FWIW, I don't see why 'a' should be treated as a command here.  It
> isn't.

No, but `view-lossage' says "Display last few input keystrokes and the
commands run."  What `read-char' comes under "input keystrokes", I
guess?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 18:41     ` Lars Ingebrigtsen
@ 2019-08-01 18:51       ` Eli Zaretskii
  2019-08-01 18:58         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-08-01 18:51 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: zkanfer@gmail.com,  21867@debbugs.gnu.org
> Date: Thu, 01 Aug 2019 20:41:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> >  C-x C-e [eval-last-sexp]
> >> >  a [char read by read-char]
> >> >  C-h l [view-lossage]
> >
> > FWIW, I don't see why 'a' should be treated as a command here.  It
> > isn't.
> 
> No, but `view-lossage' says "Display last few input keystrokes and the
> commands run."

Which is what we do: we display 'a', but we don't show any command for
it.

> What `read-char' comes under "input keystrokes", I guess?

Yes.





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 18:51       ` Eli Zaretskii
@ 2019-08-01 18:58         ` Lars Ingebrigtsen
  2019-08-01 19:18           ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-01 18:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21867, zkanfer

Eli Zaretskii <eliz@gnu.org> writes:

>> No, but `view-lossage' says "Display last few input keystrokes and the
>> commands run."
>
> Which is what we do: we display 'a', but we don't show any command for
> it.
>
>> What `read-char' comes under "input keystrokes", I guess?
>
> Yes.

Right, but view-lossage currently says

 C-x C-e ;; eval-last-sexp
 a C-h l ;; view-lossage

which is a very curious way of displaying this.  It makes it look like
"a C-h l" is a key stroke.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 18:58         ` Lars Ingebrigtsen
@ 2019-08-01 19:18           ` Eli Zaretskii
  2019-08-01 19:25             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-08-01 19:18 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 21867@debbugs.gnu.org,  zkanfer@gmail.com
> Date: Thu, 01 Aug 2019 20:58:50 +0200
> 
> >> What `read-char' comes under "input keystrokes", I guess?
> >
> > Yes.
> 
> Right, but view-lossage currently says
> 
>  C-x C-e ;; eval-last-sexp
>  a C-h l ;; view-lossage
> 
> which is a very curious way of displaying this.  It makes it look like
> "a C-h l" is a key stroke.

A newline should be enough to fix that, I think.





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 19:18           ` Eli Zaretskii
@ 2019-08-01 19:25             ` Lars Ingebrigtsen
  2019-08-02  6:41               ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-01 19:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21867, zkanfer

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: 21867@debbugs.gnu.org,  zkanfer@gmail.com
>> Date: Thu, 01 Aug 2019 20:58:50 +0200
>> 
>> >> What `read-char' comes under "input keystrokes", I guess?
>> >
>> > Yes.
>> 
>> Right, but view-lossage currently says
>> 
>>  C-x C-e ;; eval-last-sexp
>>  a C-h l ;; view-lossage
>> 
>> which is a very curious way of displaying this.  It makes it look like
>> "a C-h l" is a key stroke.
>
> A newline should be enough to fix that, I think.

Yes, that'd be OK, I think, but it means that we have to
put...  something...  into the recent_keys variable to mark the "a" as
having been read by `read-string'.  I suggested (nil . "some-string"),
but perhaps just (nil) would be OK?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-01 19:25             ` Lars Ingebrigtsen
@ 2019-08-02  6:41               ` Eli Zaretskii
  2019-08-02 11:19                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-08-02  6:41 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 21867@debbugs.gnu.org,  zkanfer@gmail.com
> Date: Thu, 01 Aug 2019 21:25:49 +0200
> 
> >>  C-x C-e ;; eval-last-sexp
> >>  a C-h l ;; view-lossage
> >> 
> >> which is a very curious way of displaying this.  It makes it look like
> >> "a C-h l" is a key stroke.
> >
> > A newline should be enough to fix that, I think.
> 
> Yes, that'd be OK, I think, but it means that we have to
> put...  something...  into the recent_keys variable to mark the "a" as
> having been read by `read-string'.  I suggested (nil . "some-string"),
> but perhaps just (nil) would be OK?

Or maybe a special symbol?





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-02  6:41               ` Eli Zaretskii
@ 2019-08-02 11:19                 ` Lars Ingebrigtsen
  2019-08-02 12:02                   ` Eli Zaretskii
  2019-08-02 20:16                   ` Stefan Monnier
  0 siblings, 2 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-02 11:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 21867, zkanfer

Eli Zaretskii <eliz@gnu.org> writes:

> Or maybe a special symbol?

The following patch does this.  Does this look like the thing to do?

The test case, with this patch, now looks like this:

 C-e			;; move-end-of-line
 C-x C-e		;; eval-last-sexp
 a 			;; 
 C-h l			;; view-lossage

I've looked at other in-tree usages of `recent-keys', and it doesn't
look like this should affect anything else.

diff --git a/lisp/help.el b/lisp/help.el
index 039d0c44e4..0cd2aecba5 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -471,6 +471,9 @@ view-lossage
 			 ((and (consp key) (null (car key)))
 			  (format ";; %s\n" (if (symbolp (cdr key)) (cdr key)
 					      "anonymous-command")))
+                         ((eq key 'non-command-character)
+                          ;; This comes from `read-char'.
+                          "\n")
 			 ((or (integerp key) (symbolp key) (listp key))
 			  (single-key-description key))
 			 (t
diff --git a/src/keyboard.c b/src/keyboard.c
index db5ca4e547..4e13bec5dd 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2091,7 +2091,6 @@ show_help_echo (Lisp_Object help, Lisp_Object window, Lisp_Object object,
 
 static Lisp_Object kbd_buffer_get_event (KBOARD **kbp, bool *used_mouse_menu,
 					 struct timespec *end_time);
-static void record_char (Lisp_Object c);
 
 static Lisp_Object help_form_saved_window_configs;
 static void
@@ -3192,7 +3191,7 @@ help_char_p (Lisp_Object c)
 
 /* Record the input event C in various ways.  */
 
-static void
+void
 record_char (Lisp_Object c)
 {
   /* quail.el binds this to avoid recording keys twice.  */
@@ -11069,6 +11068,8 @@ syms_of_keyboard (void)
 
   DEFSYM (Qundefined, "undefined");
 
+  DEFSYM (Qnon_command_character, "non-command-character");
+
   /* Hooks to run before and after each command.  */
   DEFSYM (Qpre_command_hook, "pre-command-hook");
   DEFSYM (Qpost_command_hook, "post-command-hook");
diff --git a/src/lisp.h b/src/lisp.h
index f437609fe1..ef03935b7a 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4375,6 +4375,7 @@ fast_string_match_ignore_case (Lisp_Object regexp, Lisp_Object string)
 extern void init_keyboard (void);
 extern void syms_of_keyboard (void);
 extern void keys_of_keyboard (void);
+extern void record_char (Lisp_Object);
 
 /* Defined in indent.c.  */
 extern ptrdiff_t current_column (void);
diff --git a/src/lread.c b/src/lread.c
index eec88760d4..88a27531eb 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -679,8 +679,11 @@ read_filtered_event (bool no_switch_frame, bool ascii_required,
   /* Read until we get an acceptable event.  */
  retry:
   do
-    val = read_char (0, Qnil, (input_method ? Qnil : Qt), 0,
-		     NUMBERP (seconds) ? &end_time : NULL);
+    {
+      val = read_char (0, Qnil, (input_method ? Qnil : Qt), 0,
+		       NUMBERP (seconds) ? &end_time : NULL);
+      record_char (Qnon_command_character);
+    }
   while (FIXNUMP (val) && XFIXNUM (val) == -2); /* wrong_kboard_jmpbuf */
 
   if (BUFFERP (val))

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-02 11:19                 ` Lars Ingebrigtsen
@ 2019-08-02 12:02                   ` Eli Zaretskii
  2019-08-02 20:16                   ` Stefan Monnier
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2019-08-02 12:02 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Stefan Monnier; +Cc: 21867, zkanfer

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 21867@debbugs.gnu.org,  zkanfer@gmail.com
> Date: Fri, 02 Aug 2019 13:19:01 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Or maybe a special symbol?
> 
> The following patch does this.  Does this look like the thing to do?
> 
> The test case, with this patch, now looks like this:
> 
>  C-e			;; move-end-of-line
>  C-x C-e		;; eval-last-sexp
>  a 			;; 
>  C-h l			;; view-lossage
> 
> I've looked at other in-tree usages of `recent-keys', and it doesn't
> look like this should affect anything else.

SGTM.  Stefan, any comments/objections/suggestions?





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-02 11:19                 ` Lars Ingebrigtsen
  2019-08-02 12:02                   ` Eli Zaretskii
@ 2019-08-02 20:16                   ` Stefan Monnier
  2019-08-03 12:13                     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2019-08-02 20:16 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

>   retry:
>    do
> -    val = read_char (0, Qnil, (input_method ? Qnil : Qt), 0,
> -		     NUMBERP (seconds) ? &end_time : NULL);
> +    {
> +      val = read_char (0, Qnil, (input_method ? Qnil : Qt), 0,
> +		       NUMBERP (seconds) ? &end_time : NULL);
> +      record_char (Qnon_command_character);
> +    }

Currently, the view-lossage doesn't actually say

    `a C-h l` resulted in running view-lossage

contrarily to what Lars seems to assume (and I guess, contrarily to
what the display suggests).  Instead it just says

    we saw `a` then `C-h` then `l` then view-lossage was run

I think considering this problem as specific to `read-char` is kinda wrong
(after all, `read-char` can be called as part of input-decode-map in
which case the event still very much belongs to "the events that resulted
in running a particular command"; this happens for example for
mouse-clicks in xterm-mouse-mode).

So I think maybe a better way to look at it is to add marker
pseudo-events like `end-of-command` (e.g. after running
post-command-hook), so we can discover that `a` was processed as part of
the execution of the command bound to `C-x C-e` rather than as part of
the sequence of events that triggered view-lossage.


        Stefan






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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-02 20:16                   ` Stefan Monnier
@ 2019-08-03 12:13                     ` Lars Ingebrigtsen
  2019-08-03 21:07                       ` Stefan Monnier
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-03 12:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 21867, zkanfer

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So I think maybe a better way to look at it is to add marker
> pseudo-events like `end-of-command` (e.g. after running
> post-command-hook), so we can discover that `a` was processed as part of
> the execution of the command bound to `C-x C-e` rather than as part of
> the sequence of events that triggered view-lossage.

Makes sense.  With the following patch I get:

24 5 (nil . eval-last-sexp) 97 end-of-command

in recent_keys.  `view-lossage' could use this to output this as

C-x C-e a    ;; eval-last-sexp

It does make the recent_keys list even more...  uhm...  eccentric, I
guess.  And during typical text entry, there'll be slightly more memory
usage:

72 (nil . self-insert-command) end-of-command
101 (nil . self-insert-command) end-of-command 

If this approach makes sense, I can fix up view-lossage to interpret the
new structure.

diff --git a/src/keyboard.c b/src/keyboard.c
index db5ca4e547..1a4a7d2711 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -307,6 +307,7 @@ #define GROW_RAW_KEYBUF							\
 
 static void echo_now (void);
 static ptrdiff_t echo_length (void);
+static void record_char (Lisp_Object c);
 
 /* Incremented whenever a timer is run.  */
 unsigned timers_run;
@@ -1424,6 +1425,8 @@ command_loop_1 (void)
 	      Fcons (Qnil, cmd));
 	if (++recent_keys_index >= NUM_RECENT_KEYS)
 	  recent_keys_index = 0;
+	/* Mark this as a complete command in recent_keys. */
+	record_char (Qend_of_command);
       }
       Vthis_command = cmd;
       Vreal_this_command = cmd;
@@ -1474,6 +1477,9 @@ command_loop_1 (void)
 
       safe_run_hooks (Qpost_command_hook);
 
+      /* Mark this as a complete command in recent_keys. */
+      record_char (Qend_of_command);
+
       /* If displaying a message, resize the echo area window to fit
 	 that message's size exactly.  Do this only if the echo area
 	 window is the minibuffer window of the selected frame.  See
@@ -2091,7 +2097,6 @@ show_help_echo (Lisp_Object help, Lisp_Object window, Lisp_Object object,
 
 static Lisp_Object kbd_buffer_get_event (KBOARD **kbp, bool *used_mouse_menu,
 					 struct timespec *end_time);
-static void record_char (Lisp_Object c);
 
 static Lisp_Object help_form_saved_window_configs;
 static void
@@ -11069,6 +11074,8 @@ syms_of_keyboard (void)
 
   DEFSYM (Qundefined, "undefined");
 
+  DEFSYM (Qend_of_command, "end-of-command");
+
   /* Hooks to run before and after each command.  */
   DEFSYM (Qpre_command_hook, "pre-command-hook");
   DEFSYM (Qpost_command_hook, "post-command-hook");


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-03 12:13                     ` Lars Ingebrigtsen
@ 2019-08-03 21:07                       ` Stefan Monnier
  2019-08-03 21:14                         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2019-08-03 21:07 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, zkanfer

> in recent_keys.  `view-lossage' could use this to output this as
>
> C-x C-e a    ;; eval-last-sexp

I think that would be no better than what we have now.  To be better,
we'd want something like:

    C-x C-e [eval-last-sexp] a

which clearly separates between the events received before the command
from those received during the command.


        Stefan






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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-03 21:07                       ` Stefan Monnier
@ 2019-08-03 21:14                         ` Lars Ingebrigtsen
  2019-08-03 21:49                           ` Juri Linkov
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-03 21:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 21867, zkanfer

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> in recent_keys.  `view-lossage' could use this to output this as
>>
>> C-x C-e a    ;; eval-last-sexp
>
> I think that would be no better than what we have now.  To be better,
> we'd want something like:
>
>     C-x C-e [eval-last-sexp] a
>
> which clearly separates between the events received before the command
> from those received during the command.

Yes, that's better and should be even easier to achieve.  Although we
may have to give up the columnar buffer to do that.

Did the patch otherwise make sense?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-03 21:14                         ` Lars Ingebrigtsen
@ 2019-08-03 21:49                           ` Juri Linkov
  2019-08-04 11:45                             ` Lars Ingebrigtsen
  2019-08-04 12:54                             ` Stefan Monnier
  0 siblings, 2 replies; 24+ messages in thread
From: Juri Linkov @ 2019-08-03 21:49 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, Stefan Monnier, zkanfer

>>> in recent_keys.  `view-lossage' could use this to output this as
>>>
>>> C-x C-e a    ;; eval-last-sexp
>>
>> I think that would be no better than what we have now.  To be better,
>> we'd want something like:
>>
>>     C-x C-e [eval-last-sexp] a
>>
>> which clearly separates between the events received before the command
>> from those received during the command.
>
> Yes, that's better and should be even easier to achieve.  Although we
> may have to give up the columnar buffer to do that.

The intention of the columnar format was to maintain compatibility
with the Keyboard Macro Editor (edit-last-kbd-macro).  After typing
the same keys while recording the keyboard macro produces the
buffer where `a' is presented as `self-insert-command':

  ;; Keyboard Macro Editor.  Press C-c C-c to finish; press C-x k RET to cancel.
  ;; Original keys: C-x C-e a

  Command: last-kbd-macro
  Key: none

  Macro:

  C-x C-e                 ;; eval-last-sexp
  a                       ;; self-insert-command





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-03 21:49                           ` Juri Linkov
@ 2019-08-04 11:45                             ` Lars Ingebrigtsen
  2019-08-04 21:04                               ` Juri Linkov
  2019-08-04 12:54                             ` Stefan Monnier
  1 sibling, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-04 11:45 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 21867, Stefan Monnier, zkanfer

Juri Linkov <juri@linkov.net> writes:

> The intention of the columnar format was to maintain compatibility
> with the Keyboard Macro Editor (edit-last-kbd-macro).  After typing
> the same keys while recording the keyboard macro produces the
> buffer where `a' is presented as `self-insert-command':
>
>   ;; Keyboard Macro Editor.  Press C-c C-c to finish; press C-x k RET to cancel.
>   ;; Original keys: C-x C-e a
>
>   Command: last-kbd-macro
>   Key: none
>
>   Macro:
>
>   C-x C-e                 ;; eval-last-sexp
>   a                       ;; self-insert-command

Hm...  well, that's a spanner in the works.

Is `edit-last-kbd-macro' just wrong here?  Because that "a" isn't really
a `self-insert-command', surely.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-03 21:49                           ` Juri Linkov
  2019-08-04 11:45                             ` Lars Ingebrigtsen
@ 2019-08-04 12:54                             ` Stefan Monnier
  2019-08-04 19:44                               ` Juri Linkov
  2019-08-05  9:15                               ` Lars Ingebrigtsen
  1 sibling, 2 replies; 24+ messages in thread
From: Stefan Monnier @ 2019-08-04 12:54 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Lars Ingebrigtsen, 21867, zkanfer

>   C-x C-e                 ;; eval-last-sexp
>   a                       ;; self-insert-command

This specific output would be wrong, but I guess we could output
something like:

    C-x C-e                 ;; eval-last-sexp
    a                       ;; <during eval-last-sexp>


-- Stefan






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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-04 12:54                             ` Stefan Monnier
@ 2019-08-04 19:44                               ` Juri Linkov
  2019-08-05 18:36                                 ` Stefan Monnier
  2019-08-05  9:15                               ` Lars Ingebrigtsen
  1 sibling, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2019-08-04 19:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Lars Ingebrigtsen, 21867, zkanfer

>>   C-x C-e                 ;; eval-last-sexp
>>   a                       ;; self-insert-command
>
> This specific output would be wrong, but I guess we could output
> something like:
>
>     C-x C-e                 ;; eval-last-sexp
>     a                       ;; <during eval-last-sexp>

Is it safe to assume that any characters not belonging to
a keysequence bound to a command, are read by the previous
command?  If yes, then I guess one of the patches that Lars
sent (with 'non-command-character') could be adapted
for this output.





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-04 11:45                             ` Lars Ingebrigtsen
@ 2019-08-04 21:04                               ` Juri Linkov
  0 siblings, 0 replies; 24+ messages in thread
From: Juri Linkov @ 2019-08-04 21:04 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 21867, Stefan Monnier, zkanfer

>>   Command: last-kbd-macro
>>   Key: none
>>
>>   Macro:
>>
>>   C-x C-e                 ;; eval-last-sexp
>>   a                       ;; self-insert-command
>
> Hm...  well, that's a spanner in the works.
>
> Is `edit-last-kbd-macro' just wrong here?  Because that "a" isn't really
> a `self-insert-command', surely.

A similar problem is complained about in the comments of lisp/repeat.el:

;; This works correctly inside a keyboard macro as far as recording and
;; playback go, but `edit-kbd-macro' gets it wrong.  That shouldn't really
;; matter; if you need to edit something like
;;   C-x ]              ;; forward-page
;;   C-x z              ;; repeat
;;   zz                 ;; self-insert-command * 2
;;   C-x                ;; Control-X-prefix
;; you can just kill the bogus final 2 lines, then duplicate the repeat line
;; as many times as it's really needed.  Also, `edit-kbd-macro' works
;; correctly if `repeat' is invoked through a rebinding to a single keystroke
;; and the global variable repeat-on-final-keystroke is set to a value
;; that doesn't include that keystroke.





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-04 12:54                             ` Stefan Monnier
  2019-08-04 19:44                               ` Juri Linkov
@ 2019-08-05  9:15                               ` Lars Ingebrigtsen
  1 sibling, 0 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-05  9:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 21867, zkanfer, Juri Linkov

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> This specific output would be wrong, but I guess we could output
> something like:
>
>     C-x C-e                 ;; eval-last-sexp
>     a                       ;; <during eval-last-sexp>

OK; looks good to me.  I'll do that...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-04 19:44                               ` Juri Linkov
@ 2019-08-05 18:36                                 ` Stefan Monnier
  2019-08-07 18:31                                   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2019-08-05 18:36 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Lars Ingebrigtsen, 21867, zkanfer

> Is it safe to assume that any characters not belonging to
> a keysequence bound to a command, are read by the previous
> command?

I think it's safe to say that an event read after the start of a command
and before the end of the execution of that command was received "during
<thecommand>".

The reality can be pretty complex (e.g. event can be read during
pre-command-hook, post-command-hook, recursive-edits, unrelated
process-filters, etc...), and I don't think we should strive to explain
precisely all possible cases.  The point is just to avoid misleading
output like the one under discussion.

Maybe rather than "during eval-last-sexp" we should say "during command
execution" to try and be more vague (e.g. if it's done in
post-command-hook, it's not done "during eval-last-sexp" but it's
arguably still during "during command execution").


        Stefan






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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-05 18:36                                 ` Stefan Monnier
@ 2019-08-07 18:31                                   ` Lars Ingebrigtsen
  2019-08-23  1:41                                     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-07 18:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 21867, zkanfer, Juri Linkov

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Maybe rather than "during eval-last-sexp" we should say "during command
> execution" to try and be more vague (e.g. if it's done in
> post-command-hook, it's not done "during eval-last-sexp" but it's
> arguably still during "during command execution").

That makes sense.  When I reintroduce the patch (after figuring out how
it broke macros) I'll change the text to that.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands
  2019-08-07 18:31                                   ` Lars Ingebrigtsen
@ 2019-08-23  1:41                                     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-23  1:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 21867, zkanfer, Juri Linkov

I've now fixed the kmacro problems, but there's still some
irregularities in the recorded keystrokes and commands.

Presently, if you type

(foo)RET

view-lossage will say:

 (			;; self-insert-command
 f			;; self-insert-command
 o			;; self-insert-command
 o			;; self-insert-command
 )			;; self-insert-command
 <return> <return>	;; newline-and-indent

Which is...  kinda wrong?  Because there's only one RET, but at least
we're told that `newline-and-indent' is the result, so it's not that
bad.

But with the patch set, we get:

(			;; self-insert-command
f			;; self-insert-command
o			;; self-insert-command
o			;; self-insert-command
)			;; self-insert-command
<return>		;; <during command execution>
<return>		;; newline-and-indent

which is even more wrong.

Where is that first <return> coming from?  The raw output from
`(recent-keys t)' is:

 40
 (nil . self-insert-command)
 102
 (nil . self-insert-command)
 111
 (nil . self-insert-command)
 111
 (nil . self-insert-command)
 41
 (nil . self-insert-command)
 return return
 (nil . newline-and-indent)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no






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

end of thread, other threads:[~2019-08-23  1:41 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-09  4:58 bug#21867: 25.0.50; lossage's log doesn't treat characters read by read-char as separate commands Zachary Kanfer
2019-08-01 18:05 ` Lars Ingebrigtsen
2019-08-01 18:21   ` Eli Zaretskii
2019-08-01 18:41     ` Lars Ingebrigtsen
2019-08-01 18:51       ` Eli Zaretskii
2019-08-01 18:58         ` Lars Ingebrigtsen
2019-08-01 19:18           ` Eli Zaretskii
2019-08-01 19:25             ` Lars Ingebrigtsen
2019-08-02  6:41               ` Eli Zaretskii
2019-08-02 11:19                 ` Lars Ingebrigtsen
2019-08-02 12:02                   ` Eli Zaretskii
2019-08-02 20:16                   ` Stefan Monnier
2019-08-03 12:13                     ` Lars Ingebrigtsen
2019-08-03 21:07                       ` Stefan Monnier
2019-08-03 21:14                         ` Lars Ingebrigtsen
2019-08-03 21:49                           ` Juri Linkov
2019-08-04 11:45                             ` Lars Ingebrigtsen
2019-08-04 21:04                               ` Juri Linkov
2019-08-04 12:54                             ` Stefan Monnier
2019-08-04 19:44                               ` Juri Linkov
2019-08-05 18:36                                 ` Stefan Monnier
2019-08-07 18:31                                   ` Lars Ingebrigtsen
2019-08-23  1:41                                     ` Lars Ingebrigtsen
2019-08-05  9:15                               ` Lars Ingebrigtsen

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