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