[PATCH - parser] (4) updates
A couple of patches for the parser branch:
0001: Some cleanup.
0002: The randr simulation patch from the other message.
0003: Fix function depth handling and an uninitialised function argument.
(I.e. a crash)
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 431a0709d2cbb82c18040957de33787572599702 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Wed, 17 Nov 2021 21:15:40 +0100
Subject: [PATCH 1/3] Cleanup.
---
fvwm/cmdparser.h | 6 +++---
fvwm/cmdparser_hooks.h | 14 +++---
fvwm/cmdparser_old.h | 6 +++---
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fvwm/cmdparser.h b/fvwm/cmdparser.h
index 67e6ae36..53e1d2e6 100644
--- a/fvwm/cmdparser.h
+++ b/fvwm/cmdparser.h
@@ -1,7 +1,7 @@
/* -*-c-*- */
-#ifndef CMDPARSER_H
-#define CMDPARSER_H
+#ifndef FVWM_CMDPARSER_H
+#define FVWM_CMDPARSER_H
/* included header files -- */
@@ -62,4 +62,4 @@ typedef struct
char *pos_arg_tokens[CMDPARSER_NUM_POS_ARGS];
} cmdparser_context_t;
-#endif /* CMDPARSER_H */
+#endif /* FVWM_CMDPARSER_H */
diff --git a/fvwm/cmdparser_hooks.h b/fvwm/cmdparser_hooks.h
index f05c98d3..42330246 100644
--- a/fvwm/cmdparser_hooks.h
+++ b/fvwm/cmdparser_hooks.h
@@ -1,7 +1,7 @@
/* -*-c-*- */
-#ifndef CMDPARSER_HOOKS_H
-#define CMDPARSER_HOOKS_H
+#ifndef FVWM_CMDPARSER_HOOKS_H
+#define FVWM_CMDPARSER_HOOKS_H
/* included header files -- */
@@ -46,7 +46,7 @@ typedef struct
/* returns 1 if the stored command is a module configuration command
* and 0 otherwise */
int (*is_module_config)(cmdparser_context_t *context);
- /* expandeds the command line */
+ /* expands the command line */
void (*expand_command_line)(
cmdparser_context_t *context, int is_addto, void *func_rc,
const void *exc);
@@ -55,10 +55,10 @@ typedef struct
void (*release_expanded_line)(cmdparser_context_t *context);
/* Tries to find a builtin function, a complex function or a module
* config line to execute and returns the type found or
- * CP_EXECTYPE_UNKNOWN if none of the above was identified. For a
- * builtin or a complex funtion the pointer to the description is
+ * CP_EXECTYPE_UNKNOWN if none of the above were identified. For a
+ * builtin or a complex function, the pointer to the description is
* returned in *ret_builtin or *ret_complex_function. Consumes the
- * the "Module" or the "Function" command form the input. Dos not
+ * the "Module" or the "Function" command from the input. Does not
* consider builtin functions if *ret_builtin is != NULL when the
* function is called. */
cmdparser_execute_type_t (*find_something_to_execute)(
@@ -71,4 +71,4 @@ typedef struct
void (*debug)(cmdparser_context_t *context, const char *msg);
} cmdparser_hooks_t;
-#endif /* CMDPARSER_HOOKS_H */
+#endif /* FVWM_CMDPARSER_HOOKS_H */
diff --git a/fvwm/cmdparser_old.h b/fvwm/cmdparser_old.h
index 4ce61670..d2b11b47 100644
--- a/fvwm/cmdparser_old.h
+++ b/fvwm/cmdparser_old.h
@@ -1,11 +1,11 @@
/* -*-c-*- */
-#ifndef CMDPARSER_OLD_H
-#define CMDPARSER_OLD_H
+#ifndef FVWM_CMDPARSER_OLD_H
+#define FVWM_CMDPARSER_OLD_H
/* interface functions */
/* return the hooks structure of the old parser */
const cmdparser_hooks_t *cmdparser_old_get_hooks(void);
-#endif /* CMDPARSER_OLD_H */
+#endif /* FVWM_CMDPARSER_OLD_H */
--
2.30.2
From c821293866fb8c56f00d8cee52b687097e5045a3 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Fri, 19 Nov 2021 02:09:28 +0100
Subject: [PATCH 2/3] Fake a global monitor when RandR is not available.
---
libs/FScreen.c | 69 +-
1 file changed, 52 insertions(+), 17 deletions(-)
diff --git a/libs/FScreen.c b/libs/FScreen.c
index 32ee78a7..e00af438 100644
--- a/libs/FScreen.c
+++ b/libs/FScreen.c
@@ -125,6 +125,13 @@ monitor_refresh_global(void)
monitor_global = monitor_new();
monitor_global->si = screen_info_new();
monitor_global->si->name = fxstrdup(GLOBAL_SCREEN_NAME);
+ if (!is_randr_present)
+ {
+ TAILQ_INSERT_TAIL(
+_info_q, monitor_global->si, entry);
+ TAILQ_INSERT_TAIL(
+_q, monitor_global, entry);
+ }
}
/* At this point, the global screen has been initialised. Refresh the
@@ -342,8 +349,12 @@ monitor_assign_virtual(struct monitor *ref)
void
FScreenSelect(Display *dpy)
{
- XRRSelectInput(disp, DefaultRootWindow(disp),
- RRScreenChangeNotifyMask | RROutputChangeNotifyMask);
+ if (is_randr_present)
+ {
+ XRRSelectInput(
+ disp, DefaultRootWindow(disp),
+ RRScreenChangeNotifyMask | RROutputChangeNotifyMask);
+ }
}
void
@@ -352,6 +363,10 @@ monitor_output_change(Display *dpy, XRRScreenChangeNotifyEvent *e)
XRRScreenResources *res;
struct monitor *m = NULL;
+ if (!is_randr_present)
+ {
+ return;
+ }
fvwm_debug(__func__, "%s: outputs have changed\n", __func__);
if ((res = XRRGetScreenResources(dpy, e->root)) == NULL)
Re: [RFC] Fake a global monitor when RandR is not available.
On Fri, Nov 19, 2021 at 02:14:57AM +0100, Dominik Vogt wrote:
> For debugging I need to run another fvwm in xnest, but that
> doesn't support randr.
>
> The attached patch mocks up a global monitor to use if init fails.
> It works at the first glance, but the patch is not very clean.
> Please comment.
Sorry, wrong patch, this is the correct one.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From c821293866fb8c56f00d8cee52b687097e5045a3 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Fri, 19 Nov 2021 02:09:28 +0100
Subject: [PATCH 2/3] Fake a global monitor when RandR is not available.
---
libs/FScreen.c | 69 +-
1 file changed, 52 insertions(+), 17 deletions(-)
diff --git a/libs/FScreen.c b/libs/FScreen.c
index 32ee78a7..e00af438 100644
--- a/libs/FScreen.c
+++ b/libs/FScreen.c
@@ -125,6 +125,13 @@ monitor_refresh_global(void)
monitor_global = monitor_new();
monitor_global->si = screen_info_new();
monitor_global->si->name = fxstrdup(GLOBAL_SCREEN_NAME);
+ if (!is_randr_present)
+ {
+ TAILQ_INSERT_TAIL(
+_info_q, monitor_global->si, entry);
+ TAILQ_INSERT_TAIL(
+_q, monitor_global, entry);
+ }
}
/* At this point, the global screen has been initialised. Refresh the
@@ -342,8 +349,12 @@ monitor_assign_virtual(struct monitor *ref)
void
FScreenSelect(Display *dpy)
{
- XRRSelectInput(disp, DefaultRootWindow(disp),
- RRScreenChangeNotifyMask | RROutputChangeNotifyMask);
+ if (is_randr_present)
+ {
+ XRRSelectInput(
+ disp, DefaultRootWindow(disp),
+ RRScreenChangeNotifyMask | RROutputChangeNotifyMask);
+ }
}
void
@@ -352,6 +363,10 @@ monitor_output_change(Display *dpy, XRRScreenChangeNotifyEvent *e)
XRRScreenResources *res;
struct monitor *m = NULL;
+ if (!is_randr_present)
+ {
+ return;
+ }
fvwm_debug(__func__, "%s: outputs have changed\n", __func__);
if ((res = XRRGetScreenResources(dpy, e->root)) == NULL) {
@@ -511,18 +526,19 @@ void FScreenInit(Display *dpy)
if (!XRRQueryExtension(dpy, _event, _base) ||
!XRRQueryVersion (dpy, , )) {
fvwm_debug(__func__, "RandR not present");
- goto randr_fail;
+ goto no_randr;
}
if (major == 1 && minor >= 5)
+ {
is_randr_present = true;
-
-
- if (!is_randr_present) {
+ }
+ else
+ {
/* Something went wrong. */
fvwm_debug(__func__, "Couldn't initialise XRandR: %s\n",
strerror(errno));
- goto randr_fail;
+ goto no_randr;
}
fvwm_debug(__func__, "Using RandR %d.%d\n", major, minor);
@@ -533,13 +549,21 @@ void FScreenInit(Display *dpy)
if (res == NULL || (res != NULL && res->noutput == 0)) {
XRRFreeScreenResources(res);
fvwm_debug(__func__, "RandR present, yet no outputs found.");
- goto randr_fail;
+ is_randr_present = false;
+ goto no_randr;
}
XRRFreeScreenResources(res);
scan_screens(dpy);
+ no_randr:
is_tracking_shared = false;
+ if (!is_randr_present)
+ {
+ fprintf(stderr, "Unable to initialise RandR\n");
+ monitor_refresh_global();
+ }
+
TAILQ_FOREACH(m, _q, entry) {
m->Desktops = fxcalloc(1, sizeof *m->Desktops);
m->Desktops->name = NULL;
@@ -552,10 +576,6 @@ void FScreenInit(Display *dpy)
monitor_check_primary();
return;
-
-randr_fail:
- fprintf(stderr, "Unable to initialise RandR\n");
- exit(101);
}
void
@@ -620,6 +640,10 @@ monitor_get_count(void)
struct monitor *m = NULL;
int c = 0;
+ if (!is_randr_present)
+ {
+ return 1;
+ }
TAILQ_FOREACH(m, _q, entry) {
if (m->flags & MONITOR_DISABLED)
continue;
@@ -755,7 +779,18 @@ FScreenOfPointerXY(int x, int y)
Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_scr_t screen,
int *x, int *y, int *w, int *h)
{
- struct monitor *m = FindScreen(arg, screen);
+ struct monitor *m;
+
+ if (is_randr_present)
+ {
+ m = FindScreen(arg, screen);
+ }
+ else
+ {
+ /* make sure a monitor exists */
+ monitor_check_primary();
+ m = monitor_global;
+ }
if (m == NULL) {
fvwm_debug(__func__, "%s: m is NULL\n", __func__);
return (True);
@@ -895,10 +930,10 @@ void FScreenGetResistanceRect(
Bool FScreenIsRectangleOnScreen(fscreen_scr_arg *arg, fscreen_scr_t screen,
rectangle *rec)
{
- int sx;
- int sy;
- int sw;
- int sh;
+ int sx = 0;
+ int sy = 0;
+ int sw = 0;
+ int sh = 0;
FScreenGetScrRect(arg, screen, , , , );
--
2.30.2
[RFC] Fake a global monitor when RandR is not available.
For debugging I need to run another fvwm in xnest, but that
doesn't support randr.
The attached patch mocks up a global monitor to use if init fails.
It works at the first glance, but the patch is not very clean.
Please comment.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 4201211293560a2a4f0a8636c36f3d5b37245175 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Fri, 19 Nov 2021 02:12:16 +0100
Subject: [PATCH] Fake a global monitor when RandR is not available.
---
libs/FScreen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libs/FScreen.c b/libs/FScreen.c
index f9fb11d2..e00af438 100644
--- a/libs/FScreen.c
+++ b/libs/FScreen.c
@@ -555,9 +555,9 @@ void FScreenInit(Display *dpy)
XRRFreeScreenResources(res);
scan_screens(dpy);
+ no_randr:
is_tracking_shared = false;
- no_randr:
if (!is_randr_present)
{
fprintf(stderr, "Unable to initialise RandR\n");
--
2.30.2
Re: [RFC] New parser framework for testing
On Thu, Nov 18, 2021 at 03:31:46PM +, Thomas Adam wrote: > On Thu, Nov 18, 2021 at 02:19:11PM +0100, Dominik Vogt wrote: > > Most of the tests were meant to catch parsing bugs, leaks and > > crashes. A mor organised approach in the future would be good. > > Maybe it would even be possible to generate test cases for > > commands programmatically from the BNF. > > It might be -- although my faith in our accuracy of the BNF notation we came > up with isn't too strong. Of course - the (long term) goal should also be to change the syntax of unmanageable commands so that they can be handled in BNF without much trouble. > Either way, I'll put something together which will > make it easy to expand. Thanks. > I think you're right though, Dominik, we need to pull this into a .md file and > start fleshing it out (similar to how we did the BNF work), if you're happy > for that? Indeed. > > Taking it a step further filters can be applied to *any* command > > line, not just commands: > > > > foobarfunc --match-resource "xterm" > > > > (Problem: How can we distinguish between general filters and the > > actual command/function arguments?) > > If a command in a complex function isn't overriding the calling filter, then > use that? I mean regarding parsing. If filters are universal, they shouldn't be part of the command syntax, and the parser needs to figure out which arguments are filters and which belong to the command, e.g. by a rule that filter arguments must always be placed before other arguments on the command line. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] New parser framework for testing
On Thu, Nov 18, 2021 at 02:19:11PM +0100, Dominik Vogt wrote: > Most of the tests were meant to catch parsing bugs, leaks and > crashes. A mor organised approach in the future would be good. > Maybe it would even be possible to generate test cases for > commands programmatically from the BNF. It might be -- although my faith in our accuracy of the BNF notation we came up with isn't too strong. Either way, I'll put something together which will make it easy to expand. > Sounds interesting. While implementing new ways of selecting > source/target (what is the difference?) it is still possible to > keep the existing conditionals working: If "-s ..." is present, > use it. Otherwise use the window that has been selected by a > conditional. If that's not defined either, ask the user to select > one. Yes, in that order as well, I would have thought. I'm still mulling over what I mean by source vs target, but source refers to the window or windows which the command needs to operate on, and target is the end result of that command. Take the `Move` command, for instance: Move -s -t screen|desktop|page But I think that's overcomplicating things. What matters is whether the command expects a window, or if it's a command which changes state (such as a colorset, or a setting for something). I think you're right though, Dominik, we need to pull this into a .md file and start fleshing it out (similar to how we did the BNF work), if you're happy for that? > For multi-target conditions the syntax would work too. > > Old way, loop over all windows, filter them by a resource name, > then apply a command to them: > > All (xterm) Close > > Same in new syntax (assuming "-c" marks the beginning of the > command to execute): > > All --match-resource "xterm" -c Close Something like that, yes. Although I'm suggesting that filters would allow for us doing away with commands such as: All, None, Next, Prev, as they'd become a part of the filter. So you might say: Close --filter "screen:X,desk:*,class:XTerm,condition:*" ... to close all XTerm windows (condition:*), which were only on screen X, all desks (desk:*). Either way would work though. > Taking it a step further filters can be applied to *any* command > line, not just commands: > > foobarfunc --match-resource "xterm" > > (Problem: How can we distinguish between general filters and the > actual command/function arguments?) If a command in a complex function isn't overriding the calling filter, then use that? > Note: Complex functions already have a kind of filtering with the > "I", "C", ... bits. Yeah -- this needs more thinking. Hmm. Kindly, Thomas
Re: [RFC] New parser framework for testing
We need to put the results and suggestions of this discusstion in a file. > On Thu, Nov 18, 2021 at 12:31:09AM +0100, Dominik Vogt wrote: > > Anyway, we need > > infrastructure for automated testing. > > We used to have something like that but it fell into bitrot and I removed it > years ago. Yep. > That being said, it's probably more valuable to have a set of > tests which capture the behaviour of the parser, than it is about checking > window positions, etc. Most of the tests were meant to catch parsing bugs, leaks and crashes. A mor organised approach in the future would be good. Maybe it would even be possible to generate test cases for commands programmatically from the BNF. > I see some of this as recognising that the commands need to have a common > syntax. Just dreaming up something here, but take the Move command for > example: > >Move <-- context is known or asked for, but interactive nonetheless >Move -s fvwm.next.XTerm <-- next XTerm in the ring (but interactive) >Move -s fvwm.prev.XTerm -p 200p 100p <-- prev XTerm, non-interactive > > (Here, -s indicates the *source* window). > > Resize could also work the same with with -s >... > Commands might collectively take '-s' to indicate a source, or '-t' to > indicate the destination. Be it a specific geometry, pixel/percentage, > desktop, page, etc. The syntax for these can be unified and abstracted away. >... Sounds interesting. While implementing new ways of selecting source/target (what is the difference?) it is still possible to keep the existing conditionals working: If "-s ..." is present, use it. Otherwise use the window that has been selected by a conditional. If that's not defined either, ask the user to select one. For multi-target conditions the syntax would work too. Old way, loop over all windows, filter them by a resource name, then apply a command to them: All (xterm) Close Same in new syntax (assuming "-c" marks the beginning of the command to execute): All --match-resource "xterm" -c Close Hypothetical filtering by the command itself. Call the command for each window in turn, but the command does nothing unless the --match-resource condition is true. All -c Close --match-resource "xterm" Command that works only on matching windows: Close --match-resource "xterm" etc. "All" would then work like a prefix (a la "silent", "keeprc" etc.). -- Taking it a step further filters can be applied to *any* command line, not just commands: foobarfunc --match-resource "xterm" (Problem: How can we distinguish between general filters and the actual command/function arguments?) Note: Complex functions already have a kind of filtering with the "I", "C", ... bits. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] New parser framework for testing
On Thu, Nov 18, 2021 at 12:31:09AM +0100, Dominik Vogt wrote:
> I haven't found anything yet either. Anyway, we need
> infrastructure for automated testing. That shouldn't involve much
> more than a testing directory, a Makefile with a "test" target,
> and a couple of files that can be fed into "Read" via FvwmCommand.
We used to have something like that but it fell into bitrot and I removed it
years ago. That being said, it's probably more valuable to have a set of
tests which capture the behaviour of the parser, than it is about checking
window positions, etc.
> Could you try to assemble a list of parsing test cases from past
> bug reports? We don't need hundreds but a selection of relevant
> corner cases.
Sure.
> The execution context is something different. It is basically
> meant to transport the information who or what triggered an action
> to pieces of code that need it. For example, one might need to
> know if a window button was acticated by a mouse button press, or
> a release, from the keyboard (from a menu entry etc.), from a
> module, and so forth. This change was extremely successful.
> Before we had the execution context, there were loads of transient
> bugs because code had to guess how it had been called. These
> problems are all gone now.
Indeed -- and I understood that. My point was to try and pre-enumerate this
along with the window which is required to run the action. That works fine
for individual commands but each item in a function could be working on a
different context entirely. I think what I'm trying to convey here is two
different things which I shouldn't conflate.
> The biggest problem I see is that the parsed information needs to
> be passed to the commands. I really don't want to generate C
> struct types programatically, and definitely not with odd tools
> like lex/yacc/bison. We used them for a while, and quality of the
> generated code was somewhere below zero.
OK.
> Here's an ad-hoc list of things needed for BNF* based commands:
> (* or whatever syntax description we want to use.)
>
> * A precise description of the commands' syntax in BNF.
> * A way to express alternative syntax variants. We have several
>commands that have different modes of operation; for example
>"Move" takes certain arguments in interactive mode, some
>arguments indicate noninteractive mode, and some arguments are
>shared.
I see this as the fundamental underpinings of moving things forward. Get this
right (or at the very least *standardised*), and everything becomes a lot
easier.
I see some of this as recognising that the commands need to have a common
syntax. Just dreaming up something here, but take the Move command for
example:
Move <-- context is known or asked for, but interactive nonetheless
Move -s fvwm.next.XTerm <-- next XTerm in the ring (but interactive)
Move -s fvwm.prev.XTerm -p 200p 100p <-- prev XTerm, non-interactive
(Here, -s indicates the *source* window).
Resize could also work the same with with -s
What I'm really talking myself into here is a DSL which represents the ability
to consistently apply filters to commands which operate consistently. This
would also mean conditional commands are not a separate entity, they're just
filters operating on other commands.
Commands might collectively take '-s' to indicate a source, or '-t' to
indicate the destination. Be it a specific geometry, pixel/percentage,
desktop, page, etc. The syntax for these can be unified and abstracted away.
So I think the BNF descriptions as we have them now are very useful and needed
to understand how the different commands behave now, and overlap. But for any
future changes, we need to think differently.
I don't like the overlap we have now between commands operating as
state/settings changed, and commands which all do the same thing as one
another. The BNF screams that to me even more than the man page does at this
point. ;P
> * Some way to store it in the function table (at compile time
>without making functable.h depend on all other .h files.)
Yes.
> * An indication in the function table whether a command uses old
>style parsing or BNF based.
> * A way to translate the BNF to structured data, either at
>build time (a la bison) or at run time (like the X event
>unions?).
We could start adding to the function table flags to indicate deprecation,
overlaps_with, etc. Along with parsing hints. For example (and completely
made up): imagine some command struct:
struct fvwm_command {
const char *cmd_name;
const char *syntax;
const char *other_cmd; <-- the alternate command to use if deprecated
int flags; <-- deprecated
context_t *context; <-- the window to operate on, etc.
};
So here, what I'm trying to convey is capturing the state of the command we
have now, whether it's deprecated (and the command to use instead), and some
context_t which has evaluated which
