* bug#36603: [PATCH] Simplify calling convention of describe_map @ 2019-07-11 17:34 Stefan Kangas 2019-07-11 17:37 ` Stefan Kangas 0 siblings, 1 reply; 6+ messages in thread From: Stefan Kangas @ 2019-07-11 17:34 UTC (permalink / raw) To: 36603 This patch simplifies the calling convention of describe_map in keymap.c. The immediate motivation is to simplify a set of patches that I'm preparing for Bug#8951 to rewrite substitute-command-keys in Lisp. But I think this makes sense in any case. (I'll send the patch in a separate message once this is assigned a bug number.) Thanks, Stefan Kangas ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#36603: [PATCH] Simplify calling convention of describe_map 2019-07-11 17:34 bug#36603: [PATCH] Simplify calling convention of describe_map Stefan Kangas @ 2019-07-11 17:37 ` Stefan Kangas 2019-08-20 15:32 ` Noam Postavsky 0 siblings, 1 reply; 6+ messages in thread From: Stefan Kangas @ 2019-07-11 17:37 UTC (permalink / raw) To: 36603 [-- Attachment #1: Type: text/plain, Size: 166 bytes --] Stefan Kangas <stefan@marxist.se> writes: > (I'll send the patch in a separate message once this is assigned a bug > number.) Patch attached. Thanks, Stefan Kangas [-- Attachment #2: 0001-Simplify-calling-convention-of-describe_map-Bug-3660.patch --] [-- Type: text/x-patch, Size: 3253 bytes --] From 5e35a437cb3d65919b8a547e421e69b3192c75f9 Mon Sep 17 00:00:00 2001 From: Stefan Kangas <stefankangas@gmail.com> Date: Thu, 11 Jul 2019 19:18:12 +0200 Subject: [PATCH] Simplify calling convention of describe_map (Bug#36603) * src/keymap.c (describe_map): Simplify calling convention. (describe_map_tree): Use new convention. --- src/keymap.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/keymap.c b/src/keymap.c index fc04c565a1..38be73bad8 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -89,11 +89,8 @@ static Lisp_Object store_in_keymap (Lisp_Object, Lisp_Object, Lisp_Object); static Lisp_Object define_as_prefix (Lisp_Object, Lisp_Object); -static void describe_command (Lisp_Object, Lisp_Object); -static void describe_translation (Lisp_Object, Lisp_Object); static void describe_map (Lisp_Object, Lisp_Object, - void (*) (Lisp_Object, Lisp_Object), - bool, Lisp_Object, Lisp_Object *, bool, bool); + bool, bool, Lisp_Object, bool, bool); static void describe_vector (Lisp_Object, Lisp_Object, Lisp_Object, void (*) (Lisp_Object, Lisp_Object), bool, Lisp_Object, Lisp_Object, bool, bool); @@ -2924,7 +2921,7 @@ describe_map_tree (Lisp_Object startmap, bool partial, Lisp_Object shadow, Lisp_Object prefix, const char *title, bool nomenu, bool transl, bool always_title, bool mention_shadow) { - Lisp_Object maps, orig_maps, seen, sub_shadows; + Lisp_Object maps, orig_maps, sub_shadows; bool something = 0; const char *key_heading = "\ @@ -2932,7 +2929,6 @@ describe_map_tree (Lisp_Object startmap, bool partial, Lisp_Object shadow, --- -------\n"; orig_maps = maps = Faccessible_keymaps (startmap, prefix); - seen = Qnil; sub_shadows = Qnil; if (nomenu) @@ -2999,9 +2995,8 @@ describe_map_tree (Lisp_Object startmap, bool partial, Lisp_Object shadow, sub_shadows = Fcons (XCDR (XCAR (tail)), sub_shadows); } - describe_map (Fcdr (elt), elt_prefix, - transl ? describe_translation : describe_command, - partial, sub_shadows, &seen, nomenu, mention_shadow); + describe_map (Fcdr (elt), elt_prefix, partial, transl, + sub_shadows, nomenu, mention_shadow); skip: ; } @@ -3112,9 +3107,8 @@ describe_map_compare (const void *aa, const void *bb) static void describe_map (Lisp_Object map, Lisp_Object prefix, - void (*elt_describer) (Lisp_Object, Lisp_Object), - bool partial, Lisp_Object shadow, - Lisp_Object *seen, bool nomenu, bool mention_shadow) + bool transl, bool partial, Lisp_Object shadow, + bool nomenu, bool mention_shadow) { Lisp_Object tail, definition, event; Lisp_Object tem; @@ -3122,6 +3116,12 @@ describe_map (Lisp_Object map, Lisp_Object prefix, Lisp_Object kludge; bool first = 1; + Lisp_Object seen_orig = Qnil; + Lisp_Object *seen = &seen_orig; + + void (*elt_describer) (Lisp_Object, Lisp_Object) = + transl ? describe_translation : describe_command; + /* These accumulate the values from sparse keymap bindings, so we can sort them and handle them in order. */ ptrdiff_t length_needed = 0; -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#36603: [PATCH] Simplify calling convention of describe_map 2019-07-11 17:37 ` Stefan Kangas @ 2019-08-20 15:32 ` Noam Postavsky 2019-08-20 16:00 ` Noam Postavsky 0 siblings, 1 reply; 6+ messages in thread From: Noam Postavsky @ 2019-08-20 15:32 UTC (permalink / raw) To: Stefan Kangas; +Cc: 36603 Stefan Kangas <stefan@marxist.se> writes: > + Lisp_Object seen_orig = Qnil; > + Lisp_Object *seen = &seen_orig; Do we really need the seen_orig variable? It looks like there is no reason for having seen as a pointer at all, but maybe I'm missing something. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#36603: [PATCH] Simplify calling convention of describe_map 2019-08-20 15:32 ` Noam Postavsky @ 2019-08-20 16:00 ` Noam Postavsky 2019-08-20 16:02 ` Stefan Kangas 0 siblings, 1 reply; 6+ messages in thread From: Noam Postavsky @ 2019-08-20 16:00 UTC (permalink / raw) To: Stefan Kangas; +Cc: 36603 Noam Postavsky <npostavs@gmail.com> writes: > Stefan Kangas <stefan@marxist.se> writes: > >> + Lisp_Object seen_orig = Qnil; >> + Lisp_Object *seen = &seen_orig; > > Do we really need the seen_orig variable? It looks like there is no > reason for having seen as a pointer at all, but maybe I'm missing > something. Actually, maybe the patch is wrong (i.e., over-simplifying), because it has describe_map start with a fresh seen pointer each time; but the current code passes the same pointer to each describe_map call in a loop. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#36603: [PATCH] Simplify calling convention of describe_map 2019-08-20 16:00 ` Noam Postavsky @ 2019-08-20 16:02 ` Stefan Kangas 2019-11-11 18:44 ` Stefan Kangas 0 siblings, 1 reply; 6+ messages in thread From: Stefan Kangas @ 2019-08-20 16:02 UTC (permalink / raw) To: Noam Postavsky; +Cc: 36603 Noam Postavsky <npostavs@gmail.com> writes: > > Noam Postavsky <npostavs@gmail.com> writes: > > > Stefan Kangas <stefan@marxist.se> writes: > > > >> + Lisp_Object seen_orig = Qnil; > >> + Lisp_Object *seen = &seen_orig; > > > > Do we really need the seen_orig variable? It looks like there is no > > reason for having seen as a pointer at all, but maybe I'm missing > > something. > > Actually, maybe the patch is wrong (i.e., over-simplifying), because it > has describe_map start with a fresh seen pointer each time; but the > current code passes the same pointer to each describe_map call in a > loop. Thanks for that feedback. I'll take another look and get back to you. Best regards, Stefan Kangas ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#36603: [PATCH] Simplify calling convention of describe_map 2019-08-20 16:02 ` Stefan Kangas @ 2019-11-11 18:44 ` Stefan Kangas 0 siblings, 0 replies; 6+ messages in thread From: Stefan Kangas @ 2019-11-11 18:44 UTC (permalink / raw) To: Noam Postavsky; +Cc: 36603-done Stefan Kangas <stefan@marxist.se> writes: >> Actually, maybe the patch is wrong (i.e., over-simplifying), because it >> has describe_map start with a fresh seen pointer each time; but the >> current code passes the same pointer to each describe_map call in a >> loop. > > Thanks for that feedback. I'll take another look and get back to you. I think your analysis is correct Noam, and I'm working on a different approach here, although this project is has been put on the back-burner for now. I'll get back into it once I can finish up some other things first. Thanks for taking the time to review this. Closing this bug report. Best regards, Stefan Kangas ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-11 18:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-11 17:34 bug#36603: [PATCH] Simplify calling convention of describe_map Stefan Kangas 2019-07-11 17:37 ` Stefan Kangas 2019-08-20 15:32 ` Noam Postavsky 2019-08-20 16:00 ` Noam Postavsky 2019-08-20 16:02 ` Stefan Kangas 2019-11-11 18:44 ` Stefan Kangas
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).