Re: [PATCH - parser] (4) updates
On Sun, Nov 21, 2021 at 11:41:33AM +0100, Dominik Vogt wrote: > "git add -i" is extremely useful for not accidentally committing It's "git add -pi" which I use all the time. However, this is more about me hunting down which plugin is causing this and turning it off. Kindly, Thomas
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 10:58:54PM +, Thomas Adam wrote: > On Sat, Nov 20, 2021 at 03:16:02PM +0100, Dominik Vogt wrote: > > Look at commit ba9f161998f7da942996bcf0d3f96baa8b249070. You > > added new-parser.md, but also committed a complete reindentation > > of functions.c. > > Oh heavens. That's not good at all. Clearly something has run in the > background and I had not even noticed at the point I commited that change as I > was in no way expecting anything other than that markdown file to have > changed. That's why I didn't even check. > > Sorry about that. No problem at all. This has possibly happened in earlier patches as well. There was some one-line minor whitespace change (a line containing a trailing space). I removed the trailing space, but you committed that with the whole, now empty line removed. That caused a conflict when rebasing. > I'll have to check to see how that happened. "git add -i" is extremely useful for not accidentally committing things. I've never seen anybody use is except me, and it's a bit cumbersome at first. It shows a list of all files; then I select option 5 to go through them chunk by chunk and apply or skip them. It's great for finding leftovers of experiments or debug code, and for separating topics. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 03:16:02PM +0100, Dominik Vogt wrote: > Look at commit ba9f161998f7da942996bcf0d3f96baa8b249070. You > added new-parser.md, but also committed a complete reindentation > of functions.c. Oh heavens. That's not good at all. Clearly something has run in the background and I had not even noticed at the point I commited that change as I was in no way expecting anything other than that markdown file to have changed. That's why I didn't even check. Sorry about that. I'll have to check to see how that happened. There's a possibility some "helpful" vim plugin has done this, although it hasn't done so in the past. Kindly, Thomas
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 11:17:52AM +, Thomas Adam wrote: > On Sat, Nov 20, 2021 at 10:51:51AM +0100, Dominik Vogt wrote: > > It works on my local branch but not the one in Git because of the > > reindentation commit. Can we please not reindent patches that are > > still under development? > > I haven't reindented anything -- at least, not knowingly. Look at commit ba9f161998f7da942996bcf0d3f96baa8b249070. You added new-parser.md, but also committed a complete reindentation of functions.c. > Even then, how did that cause the failure in the first place? The patch also re-ordered the includes in fvwm3.c to an order that did not work. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 10:51:51AM +0100, Dominik Vogt wrote: > It works on my local branch but not the one in Git because of the > reindentation commit. Can we please not reindent patches that are > still under development? I haven't reindented anything -- at least, not knowingly. Even then, how did that cause the failure in the first place? > I'll ditch the upstream branch for now and start a new one. Well, new-parser builds fine now, thanks. Kindly, Thomas
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 08:51:46AM +, Thomas Adam wrote: > This works, Please revert this and apply the revert patch in my other message. > but I am confused as to why it compiles fine for you: It works on my local branch but not the one in Git because of the reindentation commit. Can we please not reindent patches that are still under development? I'll ditch the upstream branch for now and start a new one. > diff --git a/fvwm/cmdparser_hooks.h b/fvwm/cmdparser_hooks.h > index 42330246b..d8ebde017 100644 > --- a/fvwm/cmdparser_hooks.h > +++ b/fvwm/cmdparser_hooks.h > @@ -3,6 +3,9 @@ > #ifndef FVWM_CMDPARSER_HOOKS_H > #define FVWM_CMDPARSER_HOOKS_H > > +#include "cmdparser.h" > +#include "functions.h" 1) Please no includes in header files. It gets really messy and unmaintainable over time. 2) Including headers from headers potentiall _breaks_ the code. Several header files work differently if some macro is defined when they are included, for example libs/FEvents.h evaluates FEVENT_PRIVILEGED_ACCESS, but since it's included by many other headers, it's more or less random whether execcontext.c includes it before defining that or after. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH - parser] (4) updates
On Fri, Nov 19, 2021 at 11:35:09PM +, Thomas Adam wrote:
> > Can you give me the error messages that cause it?
>
> See fvwm.log attached. It's possible I've missed a patch, but the code
> corresponding to this build is on the new-parser branch in git, FYI.
With the doc patch you also committed changes to functions.c that
cause this problem.
* Reindented whole file.
* Reordered include files, which causes the error.
Patch attached to back out the functions.c changes.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From e599b80d529bd75892e04d47f32558ac17018ec1 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 03:18:31 +0100
Subject: [PATCH] Revert bad functions.c reformatting.
---
fvwm/functions.c | 1170 ++
1 file changed, 667 insertions(+), 503 deletions(-)
diff --git a/fvwm/functions.c b/fvwm/functions.c
index 0754b91c..31fafa2a 100644
--- a/fvwm/functions.c
+++ b/fvwm/functions.c
@@ -26,34 +26,34 @@
#include
#endif
-#include "cmdparser.h"
-#include "cmdparser_hooks.h"
-#include "cmdparser_old.h"
-#include "commands.h"
+#include "libs/fvwm_x11.h"
+#include "libs/fvwmlib.h"
+#include "libs/charmap.h"
+#include "libs/wcontext.h"
+#include "libs/Grab.h"
+#include "libs/Parse.h"
+#include "libs/Strings.h"
+#include "libs/FEvent.h"
+#include "libs/Event.h"
+#include "fvwm.h"
+#include "externs.h"
#include "cursor.h"
-#include "events.h"
#include "execcontext.h"
-#include "expand.h"
-#include "externs.h"
#include "functable.h"
#include "functable_complex.h"
+#include "cmdparser.h"
+#include "cmdparser_hooks.h"
+#include "cmdparser_old.h"
#include "functions.h"
-#include "fvwm.h"
-#include "libs/Event.h"
-#include "libs/FEvent.h"
-#include "libs/Grab.h"
-#include "libs/Parse.h"
-#include "libs/Strings.h"
-#include "libs/charmap.h"
-#include "libs/fvwm_x11.h"
-#include "libs/fvwmlib.h"
-#include "libs/wcontext.h"
-#include "menus.h"
-#include "misc.h"
+#include "commands.h"
+#include "events.h"
#include "modconf.h"
#include "module_list.h"
-#include "repeat.h"
+#include "misc.h"
#include "screen.h"
+#include "repeat.h"
+#include "expand.h"
+#include "menus.h"
/* local definitions -- */
@@ -67,11 +67,11 @@
/* forward declarations --- */
-static void
-execute_complex_function(cond_rc_t *cond_rc, const exec_context_t *exc,
-cmdparser_context_t *pc, FvwmFunction *func, Bool has_ref_window_moved);
+static void execute_complex_function(
+ cond_rc_t *cond_rc, const exec_context_t *exc, cmdparser_context_t *pc,
+ FvwmFunction *func, Bool has_ref_window_moved);
-/* local variables */
+ /* local variables */
/* Temporary instance of the hooks functions used in this file. The goal is
* to remove all parsing and expansion from functions.c and move it into a
@@ -83,12 +83,15 @@ static const cmdparser_hooks_t *cmdparser_hooks = NULL;
/* local functions */
-static int
-__context_has_window(const exec_context_t *exc, execute_flags_t flags)
+static int __context_has_window(
+ const exec_context_t *exc, execute_flags_t flags)
{
- if (exc->w.fw != NULL) {
+ if (exc->w.fw != NULL)
+ {
return 1;
- } else if ((flags & FUNC_ALLOW_UNMANAGED) && exc->w.w != None) {
+ }
+ else if ((flags & FUNC_ALLOW_UNMANAGED) && exc->w.w != None)
+ {
return 1;
}
@@ -102,93 +105,101 @@ __context_has_window(const exec_context_t *exc, execute_flags_t flags)
* Inputs:
* cursor - the cursor to display while waiting
*/
-static Bool
-DeferExecution(exec_context_changes_t *ret_ecc,
-exec_context_change_mask_t *ret_mask, cursor_t cursor, int trigger_evtype,
-int do_allow_unmanaged)
+static Bool DeferExecution(
+ exec_context_changes_t *ret_ecc, exec_context_change_mask_t *ret_mask,
+ cursor_t cursor, int trigger_evtype, int do_allow_unmanaged)
{
- int done;
- int finished = 0;
- int just_waiting_for_finish = 0;
- Window dummy;
- Window original_w;
+ int done;
+ int finished = 0;
+ int just_waiting_for_finish = 0;
+ Window dummy;
+ Window original_w;
static XEvent e;
- Window w;
- int wcontext;
- FvwmWindow *fw;
- int FinishEvent;
-
- fw = ret_ecc->w.fw;
- w = ret_ecc->w.w;
- original_w = w;
- wcontext= ret_ecc->w.wcontext;
+ Window w;
+ int wcontext;
+ FvwmWindow *fw;
+ int FinishEvent;
+
+ fw = ret_ecc->w.fw;
+ w = ret_ecc->w.w;
+ original_w = w;
+ wcontext = ret_ecc->w.wcontext;
FinishEvent = ((fw != NULL) ? ButtonRelease : ButtonPress);
#if 1 /*!!!*/
fprintf(stderr, "!!!%s: A wc 0x%x\n", __func__, wcontext);
#endif
- if (wcontext == C_UNMANAGED && do_allow_unmanaged) {
+ if (wcontext == C_UNMANAGED && do_allow_unmanaged)
+ {
#if 1 /*!!!*/
- fprintf(stderr, "!!!%s: Bf\n", __func_
Re: [PATCH - parser] (4) updates
On Fri, Nov 19, 2021 at 11:35:09PM +, Thomas Adam wrote: > On Sat, Nov 20, 2021 at 12:23:26AM +0100, Dominik Vogt wrote: > > On Fri, Nov 19, 2021 at 03:15:43PM +, Thomas Adam wrote: > > > On Fri, Nov 19, 2021 at 03:09:35PM +, Thomas Adam wrote: > > > > On Fri, Nov 19, 2021 at 02:54:53AM +0100, Dominik Vogt wrote: > > > > > A couple of patches for the parser branch: > > > > > > > > > > 0001: Some cleanup. > > > > > 0003: Fix function depth handling and an uninitialised function > > > > > argument. > > > > > (I.e. a crash) > > > > > > > > Thanks; applied these two. > > > > > > You'll need to fix some missing #includes though, as the build's failing, > > > but > > > > Builds fine with gcc-10.2.1 in a clean source tree with > > > > $ make CFLAGS="-g3 -O3 -Wall -Werror" -j 4 > > > > Can you give me the error messages that cause it? > > See fvwm.log attached. It's possible I've missed a patch, but the code > corresponding to this build is on the new-parser branch in git, FYI. > > I know I'm being lazy, I could fix this myself, but I'd like you to check, > just in case there's something else missing which you're working on at the > same time... This works, but I am confused as to why it compiles fine for you: diff --git a/fvwm/cmdparser_hooks.h b/fvwm/cmdparser_hooks.h index 42330246b..d8ebde017 100644 --- a/fvwm/cmdparser_hooks.h +++ b/fvwm/cmdparser_hooks.h @@ -3,6 +3,9 @@ #ifndef FVWM_CMDPARSER_HOOKS_H #define FVWM_CMDPARSER_HOOKS_H +#include "cmdparser.h" +#include "functions.h" + /* included header files -- */ /* global definitions - */ Kindly, Thomas
Re: [PATCH - parser] (4) updates
On Sat, Nov 20, 2021 at 12:23:26AM +0100, Dominik Vogt wrote: > On Fri, Nov 19, 2021 at 03:15:43PM +, Thomas Adam wrote: > > On Fri, Nov 19, 2021 at 03:09:35PM +, Thomas Adam wrote: > > > On Fri, Nov 19, 2021 at 02:54:53AM +0100, Dominik Vogt wrote: > > > > A couple of patches for the parser branch: > > > > > > > > 0001: Some cleanup. > > > > 0003: Fix function depth handling and an uninitialised function > > > > argument. > > > > (I.e. a crash) > > > > > > Thanks; applied these two. > > > > You'll need to fix some missing #includes though, as the build's failing, > > but > > Builds fine with gcc-10.2.1 in a clean source tree with > > $ make CFLAGS="-g3 -O3 -Wall -Werror" -j 4 > > Can you give me the error messages that cause it? See fvwm.log attached. It's possible I've missed a patch, but the code corresponding to this build is on the new-parser branch in git, FYI. I know I'm being lazy, I could fix this myself, but I'd like you to check, just in case there's something else missing which you're working on at the same time... Kindly, Thomas /bin/sh ./config.status config.status: creating Makefile config.status: creating libs/Makefile config.status: creating fvwm/Makefile config.status: creating modules/Makefile config.status: creating bin/Makefile config.status: creating bin/FvwmCommand config.status: creating bin/FvwmPrompt/Makefile config.status: creating bin/fvwm-config config.status: creating bin/fvwm-perllib config.status: creating bin/fvwm-menu-xlock config.status: creating bin/fvwm-menu-directory config.status: creating bin/fvwm-menu-desktop config.status: creating bin/fvwm-convert-2.6 config.status: creating utils/Makefile config.status: creating perllib/Makefile config.status: creating perllib/General/Makefile config.status: creating perllib/FVWM/Makefile config.status: creating perllib/FVWM/Module/Makefile config.status: creating perllib/FVWM/Tracker/Makefile config.status: creating perllib/FVWM/Module.pm config.status: creating default-config/Makefile config.status: creating doc/Makefile config.status: creating po/Makefile config.status: creating modules/FvwmAnimate/Makefile config.status: creating modules/FvwmAuto/Makefile config.status: creating modules/FvwmBacker/Makefile config.status: creating modules/FvwmButtons/Makefile config.status: creating modules/FvwmConsole/Makefile config.status: creating modules/FvwmEvent/Makefile config.status: creating modules/FvwmForm/Makefile config.status: creating modules/FvwmIconMan/Makefile config.status: creating modules/FvwmIdent/Makefile config.status: creating modules/FvwmMFL/Makefile config.status: creating modules/FvwmPager/Makefile config.status: creating modules/FvwmPerl/Makefile config.status: creating modules/FvwmPerl/FvwmPerl config.status: creating modules/FvwmRearrange/Makefile config.status: creating modules/FvwmScript/Makefile config.status: creating modules/FvwmScript/Scripts/Makefile config.status: creating modules/FvwmScript/Widgets/Makefile config.status: creating config.h config.status: config.h is unchanged config.status: executing depfiles commands make all-recursive make[1]: Entering directory '/home/n6tadam/projects/fvwm/fvwm3' Making all in default-config make[2]: Entering directory '/home/n6tadam/projects/fvwm/fvwm3/default-config' make[2]: Nothing to be done for 'all'. make[2]: Leaving directory '/home/n6tadam/projects/fvwm/fvwm3/default-config' Making all in libs make[2]: Entering directory '/home/n6tadam/projects/fvwm/fvwm3/libs' CC gravity.o CC BidiJoin.o CC Flocale.o CC PictureUtils.o CC FScreen.o CC Bindings.o CC PictureGraphics.o CC Graphics.o CC FlocaleCharset.o CC Parse.o CC PictureImageLoader.o CC Colorset.o CC ColorUtils.o PictureImageLoader.c: In function ‘PImageLoadSvg’: PictureImageLoader.c:287:9: warning: ‘rsvg_handle_get_dimensions’ is deprecated: Use 'rsvg_handle_get_intrinsic_size_in_pixels' instead [-Wdeprecated-declarations] 287 | Frsvg_handle_get_dimensions(rsvg, &dim); | ^~~ In file included from Fsvg.h:9, from PictureImageLoader.c:46: /usr/include/librsvg-2.0/librsvg/rsvg.h:727:6: note: declared here 727 | void rsvg_handle_get_dimensions (RsvgHandle *handle, RsvgDimensionData *dimension_data); | ^~ PictureImageLoader.c:360:9: warning: ‘rsvg_handle_render_cairo’ is deprecated: Use 'rsvg_handle_render_document' instead [-Wdeprecated-declarations] 360 | Frsvg_handle_render_cairo(rsvg, cr); | ^ In file included from /usr/include/librsvg-2.0/librsvg/rsvg.h:1460, from Fsvg.h:9, from PictureImageLoader.c:46: /usr/include/librsvg-2.0/librsvg/rsvg-cairo.h:95:10: note: declared here 95 | gboolean rsvg_handle_render_cairo (RsvgHandle *handle, cairo_t *cr); | ^~~~ CC
Re: [PATCH - parser] (4) updates
On Fri, Nov 19, 2021 at 03:15:43PM +, Thomas Adam wrote: > On Fri, Nov 19, 2021 at 03:09:35PM +, Thomas Adam wrote: > > On Fri, Nov 19, 2021 at 02:54:53AM +0100, Dominik Vogt wrote: > > > A couple of patches for the parser branch: > > > > > > 0001: Some cleanup. > > > 0003: Fix function depth handling and an uninitialised function argument. > > > (I.e. a crash) > > > > Thanks; applied these two. > > You'll need to fix some missing #includes though, as the build's failing, but Builds fine with gcc-10.2.1 in a clean source tree with $ make CFLAGS="-g3 -O3 -Wall -Werror" -j 4 Can you give me the error messages that cause it? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH - parser] (4) updates
On Fri, Nov 19, 2021 at 03:09:35PM +, Thomas Adam wrote: > On Fri, Nov 19, 2021 at 02:54:53AM +0100, Dominik Vogt wrote: > > A couple of patches for the parser branch: > > > > 0001: Some cleanup. > > 0003: Fix function depth handling and an uninitialised function argument. > > (I.e. a crash) > > Thanks; applied these two. You'll need to fix some missing #includes though, as the build's failing, but I don't have the time to do so right now. Kindly, Thomas
Re: [PATCH - parser] (4) updates
On Fri, Nov 19, 2021 at 02:54:53AM +0100, Dominik Vogt wrote: > A couple of patches for the parser branch: > > 0001: Some cleanup. > 0003: Fix function depth handling and an uninitialised function argument. > (I.e. a crash) Thanks; applied these two. Kindly, Thomas
