Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Fri, Dec 17, 2021 at 01:00:53AM +, Hegel3DReloaded wrote: > On Friday, December 17th, 2021 at 1:38 AM, Dominik Vogt > wrote: > > > Should be fixed now. > > > > However, I find it really annoying that screens now have to be > > specified by name. > > Yes, now it works. So you replaced $[w.screen] with ... something as I see > in git diff. Is it [c]urrent or is it aware if it should start on exact > screen? It uses the window's screen which has already been set by the time the window is placed. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Fri, Dec 17, 2021 at 12:42:25AM +, Hegel3DReloaded wrote: > On Friday, December 17th, 2021 at 1:18 AM, Dominik Vogt > wrote: > > > When __pl_position_get_pos_simple() enconters a "center" argument > > it replaces it with DEFAULT_PLACEMENT_POS_CENTER_STRING which is > > defined to "screen $[w.screen] 50-50w 50-50w" as of commit id > > 71c57858ffebdede86c2097464339b65b5742864: > > > > > PositionPlacement: include screen for Center > > > When using "PositionPlacement Center", take into account the screen the > > > window is on, so it's centered appropriately. > > > Fixes #211 > > > This string is passed to GetMoveArguments() for interpretation. > > However, $[w.screen] never gets expanded because that line is not > > passed through the parser. FScreenGetScrRect() later looks for a > > screen with the name "$[w.screen]", finds non and falls back to > > the global monitor for placement (since a recent patch) but seems > > to have used the "current" monitor before. > > > One way to fix this woud be to replace "$[w.screen]" with "c" for > > the current screen, but that ignores the window's screen if > > specified. Also, some places of the code expect screens specified > > as "@g", " @c", "@p" while others use "g", "c" and "p". > > Are this latest examples for the "Move arguments"? G is global, c > current, and p primary. I cannot find in docs what is the difference > with or without '@'. It depends on context and is mostly undocumented. :-( If the screen is part of a single string like +0+0@g it needs the "@". Otherwise it doesn't, but maybe not everywhere. Now that screens have names screen c is harmful because you cannot name your screen "c", "g" or "p". > Passing $[w.screen] to the parser is not an > option in that part of the code? No, it's not. The patched code takes the monitor from the window structure, prints the name into a generated string, passes that to GetMoveArguments() which then looks up the original monitor structure by name. That's stupid enough without piping it through the parser. > I'm too tired now. I will try to confirm tomorrow that it used 'c' before > breaking explicit monitor placement in the current release. FindScreen() used some monitor by "accident" when it didn't find one by that name. I fixed that a while ago because of undefined behaviour and crashes. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Fri, Dec 17, 2021 at 12:13:42AM +, Hegel3DReloaded wrote: > On Friday, December 17th, 2021 at 12:18 AM, Dominik Vogt > wrote: > > On Thu, Dec 16, 2021 at 07:18:23PM +, Hegel3DReloaded wrote: > > >> xrandr --output Virtual-1 --mode 1400x1050 --primary --output Virtual-2 > >> --mode 1400x1050 --right-of Virtual-1 > > > And how do I get these virtual screens? > > Ok, I will describe my virtual environment in which I work and test. > > You need to have any Linux or *BSD system which supports > Libvirt/KVM/qemu virtualization. >... Wow, very complicated. After a bit of digging I figured out how to split a single real screen into two xrandr screens: # with a real screen of 1920x1200: $ xrandr --setmonitor top 1920/1x600/1+0+600 none $ xrandr --setmonitor bottom 1920/1x600/1+0+600 none $ xrandr --fb 1921x1200 $ xrandr --fb 1920x1200 Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Fri, Dec 17, 2021 at 01:18:17AM +0100, Dominik Vogt wrote: > On Thu, Dec 16, 2021 at 04:23:35PM +, Hegel3DReloaded wrote: > > I have opened an issue about PositionPlacement Center style: after > > the recent changes, in multi monitor mode, this centers window > > split between left and right monitor, while with latest fvwm3 > > release, this was not the case. Using dv/pager-noaspect where the > > current issues have been resolved couple days ago. It is > > repeatable on the current master. > > Actually that never worked, it only looked like it did - by > accident. > > When __pl_position_get_pos_simple() enconters a "center" argument > it replaces it with DEFAULT_PLACEMENT_POS_CENTER_STRING which is > defined to "screen $[w.screen] 50-50w 50-50w" as of commit id > 71c57858ffebdede86c2097464339b65b5742864: > > >PositionPlacement: include screen for Center > > > >When using "PositionPlacement Center", take into account the screen the > >window is on, so it's centered appropriately. > > > >Fixes #211 > > This string is passed to GetMoveArguments() for interpretation. > However, $[w.screen] never gets expanded because that line is not > passed through the parser. FScreenGetScrRect() later looks for a > screen with the name "$[w.screen]", finds non and falls back to > the global monitor for placement (since a recent patch) but seems > to have used the "current" monitor before. > > One way to fix this woud be to replace "$[w.screen]" with "c" for > the current screen, but that ignores the window's screen if > specified. Also, some places of the code expect screens specified > as "@g", " @c", "@p" while others use "g", "c" and "p". Should be fixed now. However, I find it really annoying that screens now have to be specified by name. style xterm startsonscreen DVI-I-1 is completely non-portable. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Thu, Dec 16, 2021 at 04:23:35PM +, Hegel3DReloaded wrote: > I have opened an issue about PositionPlacement Center style: after > the recent changes, in multi monitor mode, this centers window > split between left and right monitor, while with latest fvwm3 > release, this was not the case. Using dv/pager-noaspect where the > current issues have been resolved couple days ago. It is > repeatable on the current master. Actually that never worked, it only looked like it did - by accident. When __pl_position_get_pos_simple() enconters a "center" argument it replaces it with DEFAULT_PLACEMENT_POS_CENTER_STRING which is defined to "screen $[w.screen] 50-50w 50-50w" as of commit id 71c57858ffebdede86c2097464339b65b5742864: >PositionPlacement: include screen for Center > >When using "PositionPlacement Center", take into account the screen the >window is on, so it's centered appropriately. > >Fixes #211 This string is passed to GetMoveArguments() for interpretation. However, $[w.screen] never gets expanded because that line is not passed through the parser. FScreenGetScrRect() later looks for a screen with the name "$[w.screen]", finds non and falls back to the global monitor for placement (since a recent patch) but seems to have used the "current" monitor before. One way to fix this woud be to replace "$[w.screen]" with "c" for the current screen, but that ignores the window's screen if specified. Also, some places of the code expect screens specified as "@g", " @c", "@p" while others use "g", "c" and "p". Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Thu, Dec 16, 2021 at 07:18:23PM +, Hegel3DReloaded wrote: > xrandr --output Virtual-1 --mode 1400x1050 --primary --output Virtual-2 > --mode 1400x1050 --right-of Virtual-1 And how do I get these virtual screens? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [bug] Style PositionPlacement Center confused in multi monitor setup
On Thu, Dec 16, 2021 at 04:23:35PM +, Hegel3DReloaded wrote: > I have opened an issue about PositionPlacement Center style: after the recent > changes, > in multi monitor mode, this centers window split between left and right > monitor, while with > latest fvwm3 release, this was not the case. Using dv/pager-noaspect where > the current > issues have been resolved couple days ago. It is repeatable on the current > master. You forgot instructions to reproduce. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Dec 13, 2021 at 08:06:41AM +, Hegel3DReloaded wrote: > It appears that on "Restart" Front Panel ends up in the +0 +0, upper left > corner of the screen. It starts ok, it is placed at the bottom center > part, but something relocates it in the last moment. I remember we had this > problem often introduced and then fixed again. > > Now it seems that > > "All (FrontPanel,CirculateHit) PlaceAgain" is causing the problem. That is, > it behaves differently that on fvwm3 1.0.4 and fvwm 2.6.9. It looks like > PlaceAgain is ignoring Style for panel: > > PositionPlacement $[infostore.frontpanel.pos.placement] > > ... where infostore.frontpanel.pos.placement is "screen c 50-50w -0p ewmhiwa" Oh, I had noticed that but thought it was something to fix in the move_loop rewrite. It's fixed now. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Dec 13, 2021 at 07:40:06AM +, Hegel3DReloaded wrote: > On Monday, December 13th, 2021 at 2:14 AM, Dominik Vogt > wrote: > > On Mon, Dec 13, 2021 at 12:22:47AM +, Hegel3DReloaded wrote: > > > > Is this has something to do with intensive work you and Thomas did > > > couple weeks ago with new parser? > > > > Yes, I fixed a leak at the cost of introducing a crash. > > I see. It is working now. May I close > https://github.com/fvwmorg/fvwm3/issues/647 Ye, please. > or you will do it with some description FTR? Only if forced to. I'm not good with web tools. :-) Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Dec 13, 2021 at 12:32:53AM +, Hegel3DReloaded wrote: > On Monday, December 13th, 2021 at 1:18 AM, Dominik Vogt > wrote: > > On Mon, Dec 13, 2021 at 12:12:30AM +, Hegel3DReloaded wrote: > > > > "Move +32768p +32768p", > > > Side note: The maximum valid position is +32767p +32767p. This > > probably wraps around to -32768. > > That's it! If I redefine this to +32767p +32767p it starts to work as > expected. > This will probably be ok to change no matter if used with FVWM 2 or FVWM 3. > Something has changed in the new code apparently. I've committed a patch to master that truncates out of range sizes and positions to the max/min values instead of rejecting them. Is there anything else in your config that's out of range? The X protocol is limited to 16 bit values for positions (signed) and window sizes (unsigned). I.e. -32768 to +32767 and 0 to 65535. > but never mind, reducing it to 32767 fixes it. -32768 is probably the better choice for "out of view" windows. Fvwm doesn't use negative coordinates. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Dec 13, 2021 at 12:22:47AM +, Hegel3DReloaded wrote: > > I'm not very happy with the "new" monitor handling code. It added > > potentially broken new parsing code all over the place. That > > should really be done in a single, well tested parser. At the > > moment we have a myriad of places where parsing is done, and its > > very tedious to even execute all code paths. > > Is this has something to do with intensive work you and Thomas did > couple weeks ago with new parser? Yes, I fixed a leak at the cost of introducing a crash. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Dec 13, 2021 at 12:12:30AM +, Hegel3DReloaded wrote: > On Monday, December 13th, 2021 at 12:46 AM, Hegel3DReloaded > wrote: > > On Sunday, December 12th, 2021 at 12:40 PM, Dominik Vogt > > dominik.v...@gmx.de wrote: > > > > The dv/pager-noaspect should work now, but I've not tested it with > > > multiple monitors. > > Now I have iteresting behaviour with my "Local" pager even before I started > to test: when Local pager does not posess pointer, it should disappear by > the means of > "Move +32768p +32768p", Side note: The maximum valid position is +32767p +32767p. This probably wraps around to -32768. > but instead of doing this, it changes > my pointer into cross, GeometryWindow appears, and expects from me to move > pager around! :-) Can you give me the relevant parts of the config and some instructions, please? > It seems to me it looses the window X11 resource context of "LocalPager" [1] > Schedule Periodic $[infostore.rootpagertimeout] 131313 All (LocalPager, > CirculateHit, !HasPointer, !State 6) f_HideLocalPager > https://github.com/NsCDE/NsCDE/blob/master/data/fvwm/Functions.fvwmconf.in Is that the config file? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sun, Dec 12, 2021 at 11:46:05PM +, Hegel3DReloaded wrote: > On Sunday, December 12th, 2021 at 12:40 PM, Dominik Vogt > wrote: > > > On Sat, Dec 04, 2021 at 02:04:48PM +, Hegel3DReloaded wrote: > > >> Sounds ok. Which branch should I checkout to test this? > > > The dv/pager-noaspect should work now, but I've not tested it with > > multiple monitors. > Sorry to say, but I even didn't have a chance to start after trying to log in. > It crashes badly. Something to do with malloc and pointers. Should be fixed. We need test cases for all this parsing code. :-/ I'm not very happy with the "new" monitor handling code. It added potentially broken new parsing code all over the place. That should really be done in a single, well tested parser. At the moment we have a myriad of places where parsing is done, and its very tedious to even execute all code paths. > When we get pass this point, I will test the rest you wrote. BTW, I'm actually > using one big monitor, but when testing multiple monitors setup, I'm helping > myself with KVM/Qemu virtual machines with virtual monitors. The crash > mentioned above was still in a single monitor setup. I didn't get very far. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 02:04:48PM +, Hegel3DReloaded wrote: > On Sat, Dec 04, 2021 at 02:13:06PM +0100, Dominik Vogt wrote: > >> On Sat, Dec 04, 2021 at 02:03:46PM +0100, Dominik Vogt wrote: > > >> Maybe the scaling argument can be set to "auto" or something to > >> keep the desktopscale within reasonable limits? (The example with > >> desktopsize 50x1 earlkier in the discussion showed that the pager > >> currently does not have a reasonable scaling algorithm). > > >> Maybe something like this by default? > > >> desktopscale auto [max%] > > >> "auto" means to make the pager consume "max%" of the desktop width > >> and height. The default would be "auto 10" or something, so (a) > >> by default the pager autoscales and (b) it always consumes 10% of > >> the screen size in one direction and no more than 10% in the other > >> direction. > > > Experimental patch attached (only initial size calculation; no > > live resizing yet). > > Sounds ok. Which branch should I checkout to test this? The dv/pager-noaspect should work now, but I've not tested it with multiple monitors. By default, the pager uses no more than 25% of the screen size (the root window size) in both directions (and tries to maximize within these limits), i.e. the default is AutoScale 25 When the desktop is resized with the "desktopsize" command or by adding or removing monitors, the window is resized within the given limits to match the aspect ratio. (That happens even if the user has manually resized the pager window). Things to test: * When the pager window is created or the desktop layout changes, the dimensions of the *current* desk are used for size calculations. I'm not sure if there are unwanted effects if the pointer is on a different screen than the pager window when that happens. Is this still something one would have with Xrandr? * Multi-monitor setup with one screen, i.e. one fvwm manages all monitors. * Multi-monitor setup with multiple screens, a separate fvwm instance runs for each screen. * Add and remove monitors in each setup. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 02:04:48PM +, Hegel3DReloaded wrote: > > Experimental patch attached (only initial size calculation; no > > live resizing yet). > > Sounds ok. Which branch should I checkout to test this? Try out dv/pager-noaspect. It contains more work to allow resizing the window when the desktop layout is changed, but it doesn't work yet. The resize_pager_window() call from list_new_page() doesn't work. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 02:13:06PM +0100, Dominik Vogt wrote:
> On Sat, Dec 04, 2021 at 02:03:46PM +0100, Dominik Vogt wrote:
> > Maybe the scaling argument can be set to "auto" or something to
> > keep the desktopscale within reasonable limits? (The example with
> > desktopsize 50x1 earlkier in the discussion showed that the pager
> > currently does not have a reasonable scaling algorithm).
>
> Maybe something like this by default?
>
> desktopscale auto [max%]
>
> "auto" means to make the pager consume "max%" of the desktop width
> and height. The default would be "auto 10" or something, so (a)
> by default the pager autoscales and (b) it always consumes 10% of
> the screen size in one direction and no more than 10% in the other
> direction.
Experimental patch attached (only initial size calculation; no
live resizing yet).
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From f82e4ce6df30513d8073f8cda02525aa36ba3464 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 4 Dec 2021 14:49:58 +0100
Subject: [PATCH] Pager autoscale draft.
---
modules/FvwmPager/FvwmPager.c | 11 +--
modules/FvwmPager/FvwmPager.h | 3 ++-
modules/FvwmPager/x_pager.c | 26 --
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/modules/FvwmPager/FvwmPager.c b/modules/FvwmPager/FvwmPager.c
index 52199ce9..f0a60dd1 100644
--- a/modules/FvwmPager/FvwmPager.c
+++ b/modules/FvwmPager/FvwmPager.c
@@ -1792,7 +1792,8 @@ void ParseOptions(void)
FvwmPictureAttributes fpa;
Scr.FvwmRoot = NULL;
Scr.Hilite = NULL;
- Scr.VScale = 32;
+ Scr.Scale = 10;
+ Scr.do_autoscale = 1;
fpa.mask = 0;
if (Pdepth <= 8)
@@ -2257,7 +2258,13 @@ void ParseOptions(void)
}
else if (StrEquals(resource, "DeskTopScale"))
{
- sscanf(arg1,"%d",);
+ sscanf(arg1,"%d",);
+ Scr.do_autoscale = 0;
+}
+else if (StrEquals(resource, "AutoScale"))
+{
+ sscanf(arg1,"%d",);
+ Scr.do_autoscale = 1;
}
else if (StrEquals(resource, "WindowColors"))
{
diff --git a/modules/FvwmPager/FvwmPager.h b/modules/FvwmPager/FvwmPager.h
index 85805240..e371d4ed 100644
--- a/modules/FvwmPager/FvwmPager.h
+++ b/modules/FvwmPager/FvwmPager.h
@@ -70,7 +70,8 @@ typedef struct ScreenInfo
char *Hilite; /* the fvwm window that is highlighted
* except for networking delays, this is the
* window which REALLY has the focus */
- unsigned VScale; /* Panner scale factor */
+ unsigned Scale;/* Panner scale factor */
+ int do_autoscale : 1;
Pixmap sticky_gray_pixmap;
Pixmap light_gray_pixmap;
Pixmap gray_pixmap;
diff --git a/modules/FvwmPager/x_pager.c b/modules/FvwmPager/x_pager.c
index b2f22a62..a30fcf4f 100644
--- a/modules/FvwmPager/x_pager.c
+++ b/modules/FvwmPager/x_pager.c
@@ -695,10 +695,32 @@ void initialize_pager(void)
} else if (pwindow.height > label_h * Rows) {
pwindow.width = ((pwindow.height - label_h * Rows + Rows) *
vWidth) / vHeight + Columns;
+ } else if (Scr.do_autoscale) {
+ int w_raw;
+ float wf;
+ int h_raw;
+ float hf;
+ rectangle screen_g;
+ float f;
+
+ FScreenGetScrRect(
+ NULL, FSCREEN_CURRENT, _g.x, _g.y,
+ _g.width, _g.height);
+ w_raw = VxPages * vWidth * Columns + Columns;
+ h_raw = VyPages * vHeight * Rows + Rows;
+ wf =
+ ((float)w_raw) /
+ (screen_g.width * ((float)Scr.Scale) / 100.0);
+ hf =
+ ((float)h_raw) /
+ (screen_g.height * ((float)Scr.Scale) / 100.0);
+ f = (wf > hf) ? wf : hf;
+ pwindow.width = ((float)w_raw) / f;
+ pwindow.height = ((float)h_raw) / f;
} else {
- pwindow.width = (VxPages * vWidth * Columns) / Scr.VScale +
+ pwindow.width = (VxPages * vWidth * Columns) / Scr.Scale +
Columns;
- pwindow.height = (VyPages * vHeight * Rows) / Scr.VScale +
+ pwindow.height = (VyPages * vHeight * Rows) / Scr.Scale +
label_h * Rows + Rows;
}
}
--
2.30.2
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 02:03:46PM +0100, Dominik Vogt wrote: > Maybe the scaling argument can be set to "auto" or something to > keep the desktopscale within reasonable limits? (The example with > desktopsize 50x1 earlkier in the discussion showed that the pager > currently does not have a reasonable scaling algorithm). Maybe something like this by default? desktopscale auto [max%] "auto" means to make the pager consume "max%" of the desktop width and height. The default would be "auto 10" or something, so (a) by default the pager autoscales and (b) it always consumes 10% of the screen size in one direction and no more than 10% in the other direction. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 11:14:09AM +, Hegel3DReloaded wrote: > On Saturday, December 4th, 2021 at 11:39 AM, Dominik Vogt > wrote: > > > Okay, that is a real problem. The pager definitely needs to react > > to changes of the desktop layout, be it by attaching or detaching > > monitors at run time or by issuing the desktopsize command. > > > > The pager should fix that situation itself and not expect the fvwm > > core to do it. There are two cases and multiple possible ways to deal > > with it: > > > > 1) Pager is using a user specified geometry: > > > >a) Stick with the user specified size. > >b) Resize the window proportionally. > > > > 2) Pager is using the calculated default size: > > > >a) Recalculate total size and use that. > >b) Resize the window proportionally but try to keep it close > > to the current one; at least don't make it much bigger or > > smaller. > > > > The module interface provides the information necessary to > > implement that. > > > > Would 1a + 2b be sensible default behaviour? > > By "calculated default size" do you mean DeskTopScale FvwmPager config > paremater? I mean the size it would use when started as a standalone window without any explicit geometry configuration present. > If yes, then I think yes, 1a if user said so with Geometry parameter, > no matter what we think of this. > 2b is a bit of gray area. I can > imagine some border cases where huge change of resolution or attaching > notebook to the 3 monitors in a row docking station in mode where > FvwmPager doesn't have "Monitor" config directive can give odd results. Yes, it probably needs some configuration by the user. > If 2b will have logic or more precisely heuristics similar to this: > > "if I need to grow more than twice my current size in X or Y direction, > apply some divisor to make it a bit less space hungry (the result to be > 1.5 instead of 2 for example), if size is more than 4 times, apply even > bigger divisor, and the last safety which should never be hit unless you > emit your screen on some open space expensive concert screen wall with > insane resolution, stop growing if the resulting pager will be bigger > then XY in proportion to $[vp.width] and/or $[vp.height]". Hmmm. Rules of thumb if no config is present: * If you switch the desktop layout between two geometries A and B multiple times, the result should alays be the same. * If you initially start the pager P1 with desktop geometery A then switch to geometry B and start another pager P2, both pagers should have the same dimensions. I guess it's only possible to stick to both rules at the same time if the pager geometry is completely recalculated when the desktop geometry changes? Maybe the scaling argument can be set to "auto" or something to keep the desktopscale within reasonable limits? (The example with desktopsize 50x1 earlkier in the discussion showed that the pager currently does not have a reasonable scaling algorithm). > This can be tested in KVM/qemu xrandr changing sharply from 1024x768 to > 2560x1440 for example, and with configuring RandR to have 3-4 Virtual > monitors in one row. Changing the desktopsize at run time also works. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Rename back to fvwm.
On Sat, Dec 04, 2021 at 10:24:23AM +, Hegel3DReloaded wrote:
> On Saturday, December 4th, 2021 at 11:00 AM, Dominik Vogt
> wrote:
>
> > Fvwm is the only project I'm aware of that changes its name with
> > each major version. Let's _please_ go back to "fvwm" and replace
> > the 1.x versioning scheme with 3.x. Automake + autoconf can deal
> > with renaming executables if distros need that. Both versions use
> > the same config files anyway. You cannot have version 2 and 3
> > installed at the same time using the default config file.
>
> > Some people still insist on calling fvwm-2.6.x "fvwm2". Why? The
> > name has been "fvwm" officially since April 2002.
>
> > The name split caused extra work and confusion back in ancient
> > times. The same thing will happen again.
>
> As much as I may agree with this opinion, I think this shouldn't
> be performed now when it is already fvwm3 for a longer time. It
> is already packaging that way, and by trying to fix this can cause
> counter effect, that is, introduce even more confusion.
Exactly the same self-induced situation as with the name of
version 2. We can still change it before version 3 becomes part
of the major distros. After that it will become more complicated.
> As of old confusion, it will be here forever. Some Linux distros are
> packaging FVWM 2.X as "fvwm2", and some BSD variants are even shipping
> with ancient fvwm1 in the base, and providing "fvwm2" from the ports.
Yes, just like many other packages where old versions for the
distros still provide old versions. For example gcc8/gcc9/gcc10
etc. Distros have a way to deal with this situation.
> Weather main binary is called "fvwm" or "fvwm2", or having the same
> name, one beeing in /usr/bin, the other in /usr/local/bin or wherever
> is the current case.
>
> It is not really a horror, but having something that started as "fvwm3"
> as a name, going back to fvwm-3.X will be really another level of
> confusion on the top of this "mild" confusion mentioned above.
As the person who made the patches to rename version 2 to "fvwm",
I disagree. It caused problems because there were lots of
inconsistencies in the code and in documentation. People didn't
understand that version 1 ("fvwm1" in some distros) wasn't
anything that a normal user would still want.
> As they were packing fvwm1 or fvwm, fvwm or fvwm2, let them pack
> fvwm3 I say.
Yes. Package maintainers can do that regardless of whether the
project name is "fvwm" or just "fvwm".
> And last but not least, it is not som much the name IMHO, as about
> quality and the power this wonderful piece of software gives to
> those who know how to appreciate this.
Okay, I don't understand this one. I'd imagine people could
appreciate version 3 more if they are aware that it's still the
same software being developed by the same people and not a fork
created over some dispute.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Dec 04, 2021 at 10:09:34AM +, Hegel3DReloaded wrote: > Yes, there are some cases, or rather there > will be them in FVWM3 when it will approach the point where resolution > change, or new monitor addition will not mandate fvwm restart to get things > in place. In this case, FvwmPager can end up smaller or bigger than wanted. Okay, that is a real problem. The pager definitely needs to react to changes of the desktop layout, be it by attaching or detaching monitors at run time or by issuing the desktopsize command. The pager should fix that situation itself and not expect the fvwm core to do it. There are two cases and multiple possible ways to deal with it: 1) Pager is using a user specified geometry: a) Stick with the user specified size. b) Resize the window proportionally. 2) Pager is using the calculated default size: a) Recalculate total size and use that. b) Resize the window proportionally but try to keep it close to the current one; at least don't make it much bigger or smaller. The module interface provides the information necessary to implement that. Would 1a + 2b be sensible default behaviour? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
I've committed two patches.
1) fac9eb945e442bebfb2612fed8edb68a9853a6ca
Removes minimum size, base size and size increment hints from the
pager. This change has not been disputed by anybody.
2) ff3afd31466f604ee5a3711486d1b1da2ee2b090
Also removes the aspect ratio hint. this patch could be reverted
separately from the other one.
I understand that not everybody is happy with that. Reasons for
this decision:
* Nobody who advocated the aspect ratio actually resizes pagers
or knows anybody who does. Thus, the usability gain remains
speculative.
* The aspect ratio has real, existing consequences on pagers
started by Fvwmbuttons:
(a) Swallowed pagers don't use up all available space,
(b) animation of pagers used as panels is broken ("sliding").
(* Resizing windows with aspect ratio has some usability problems.
these may become less important when "elastic paging" is
implemented.)
* There's a workaround for (a): Explicitly give the pager a
geometry string in its configuration.
Disadvantage: If the swallowed pager just uses the default pager
config or the same config as standalone pagers, the
configuration change is big. You have to create a whole new
module config block for the swallowed pager.
* No workaround is available for (b), and nobody had an idea how
to fix it and still have aspect ratio hints. Giving the pager
an explicit geometry deos not work for panels that are supposed
to use the default size.
(a) and (b) are real bugs that break my config.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Rename back to fvwm.
Fvwm is the only project I'm aware of that changes its name with each major version. Let's _please_ go back to "fvwm" and replace the 1.x versioning scheme with 3.x. Automake + autoconf can deal with renaming executables if distros need that. Both versions use the same config files anyway. You cannot have version 2 and 3 installed at the same time using the default config file. Some people still insist on calling fvwm-2.6.x "fvwm2". Why? The name has been "fvwm" officially since April 2002. The name split caused extra work and confusion back in ancient times. The same thing will happen again. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Snap rewrite #2
On Thu, Dec 02, 2021 at 11:25:07AM -0700, Jaimos Skriletz wrote: > On Thu, Dec 2, 2021 at 10:46 AM Dominik Vogt wrote: > > > > This is about an accident during elastic paging development, but > > it feels "good" to me. I've secretly always hated the "snapping" > > effect of snap attraction. Windows jumping around is confusing, > > especially if the distance is big, e.g. if you use a "100" edge > > resistance. So here is the new idea: > > > > I like it when my window snap to edges/other windows. Though I also > like this idea as well, I can see both being useful. Is this going to > be configurable so a user can use the behavior they prefer? It could be configurable at the cost of complete code duplication. The existing method is only aware of the window's requested position. The new method needs to keep two window positions, the position where it was blocked and the position where it would be if there were no blocking. Apart from the visual annoyance, the conventional method has some usability problems because it's impossible to place windows a few pixels away from other windows, or a little bit over the screen edge. (Of course you can do that with the Alt key, but few people know about the keybindings at all). Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Snap rewrite #2
This is about an accident during elastic paging development, but it feels "good" to me. I've secretly always hated the "snapping" effect of snap attraction. Windows jumping around is confusing, especially if the distance is big, e.g. if you use a "100" edge resistance. So here is the new idea: When one window is pushed against another window's border or some other barrier (screen edge, whatever): A +B + C + *| |* | * -+|-+| -+ |+ |+ +|--- -+ -+ -+ -> -> -> It stops in that position while the pointer moves on (* in the image), image B. When the pointer finally moves too far, the window does not jump towards the pointer position but simply resumes moving as if this "blocking" never happened (image C). When moving the window back to the left, it just moves freely without being blocked in any place. Think of the other windows border as a jammed door: To get through, you have to push. This causes pressure, and when that's hight enough, the door opens. Going back is easy because the door is already open. And if you can give up pushing at any time and just walk back. -- To push a window past the screen edge, just keep pushing. Once the edge's resistance is overcome, the window moves as if nothing had happened. When you pull it back, it won't snap back to the screen unless you push it aginst the border again. -- This is now all possible because the elastic paging code keeps the pointer in the middle of the screen (invisible). There may be some tricky situations when top and bottom border are both near obstacles. -- To get a feeling for the new code (maybe 40% complete), check the latest version of the branch "dv/elastic-paging-prototype". It doesn't have paging yet or the new "snap" method. But do try to move windows over screen edges and try pressing Shift/Control during mouse movement. With Shift the window moves much faster (factor 8) and with Conrol slower (factor 0.5). Now that the fixed offset between pointer and window position doesn't exist anymore, it's easy to do things like this. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Tue, Nov 30, 2021 at 02:51:49PM +, Hegel3DReloaded wrote: > > The situation is: > > 1. It's implementation is more or less broken everywhere. => > > Nobody uses it. > > > 2. If the ratio is not close to 1, it's outright annoying or even > > unusable. Try this: > > > > desktopsize 50x1 > > FvwmPager > > all (FvwmPager) move 0 50 > > > a) Grab the top border and resize it by moving the pointer. > > => Maximum height stops around "WidthX412" because the width > > automatically grows bacause af the aspect ratio, but can > > not get bigger than 32767. > > BAD > > Since the ways of using FvwmPager are so different between people's ideas how > to set it up, I think that having the opportunity to configure keeping aspect > ratio makes a sense. Those who are using 50x1 DesktopSize will of course not > use that obviously, but people who use 2x2, 3x2, 2x3, 4x2 and similar and not > having it swallowed inside FvwmButtons may prefer to keep aspect ratio. There > can even be here a solution like in drawing programs: you press Ctrl or Shift > while resizing the object to keep aspect ratio, otherwise, it is free form. > > [...] > > > Summary: There are a lot of quirks and bugs. Is there any gain > > in usability to make up for this? > > Maybe having ctrl+mouse-resize as I proposed above? Okay, I can imagine rare cases when one wants to re-enforce a multiple of the original aspect ratio, and I'd be fine with another key combination to force it. However, what are we supposed to do with windows with a legitimate, fulltime aspect ratio like mplayer? It would be very inconvenient to have the a. r. ignored by default. Unfortunately I see no working heuristics to tell a "hard" a. r. from a "hint". > > > But in terms of configurability, I still think an option > > > where the user can decide to preserve the aspect ratio when resizing > > > the window or not allows the most freedom for users. > > > But why would anybody want that? Does anybody really use a > > standalone pager, and if so, ever resizes it? > > I'm using 3 kinds of standalone FvwmPagers > > - "Local" Pager - showing only the current desk > - "Desk" Pager - showing what is on the non-current desk I select on the > Front Panel > - "Global" Pager - all desks and pages > > They are floating semi-transient objects. That is, having them transient is > not enough for me, so they quickly disappear by Schedule if they don't posess > the pointer. > > I don't really resize them, :-) > the are made big enough by dinamically following screen resolution, Isn't that what everybody does - give it a good default size and that's it? What kind of use case is that; someone thinks the current size is too small and wants to make it 50% bigger? Or the pager is in the way, and instead of closing, lowering, shading, iconifying or sending it to some container one makes the window smaller? What is so special about the pager that it Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Mon, Nov 29, 2021 at 01:02:04PM -0700, Jaimos Skriletz wrote: > If you set a geometry, *FvwmButtons: Geometry 100x100, the aspect > ratio hints do not get set (as in the default-config), and you can > resize the window to be any size. Yes, but that doesn't help for the FvwmButtons panel. The panel is just a normally sized FvwmPager. I don't want to tell it a geometry. It shall open with the standard size whenever used. > In conclusion, if the aspect ratio hints also go it won't affect me > directly. Still, what is the _benefit_ of having an aspect ratio set? The situation is: 1) It's implementation is more or less broken everywhere. => Nobody uses it. 2) If the ratio is not close to 1, it's outright annoying or even unusable. Try this: desktopsize 50x1 FvwmPager all (FvwmPager) move 0 50 a) Grab the top border and resize it by moving the pointer. => Maximum height stops around "WidthX412" because the width automatically grows bacause af the aspect ratio, but can not get bigger than 32767. BAD => Every time you try you get a different height: 412, 411, 330, 359 etc. BAD. b) Now make it smaller by resizing it downwards from the rop border. => When the top border comes close to the bottom border, the window shrinks really fast. CONFUSING => Assume you have reduced it to a tiny rectangle at the left side of the screen (xmag says its 58x5 pixels, incldung borders). Pull the border down further. The window jumps back to full size, from 58x5 to 9750x123 at the original position. VERY CONFUSING => Repeat that, but drag the pointer quickly up and down to the screen border. THe pager window ends up with a random height. BAD => If you're unlucky, while the window shrinks the top right edge may end up below the cursor. Fvwm will then automatically grab that corner for further resizing. VERY CONFUSING c) But things are even worse. Create a fresh pager and grab it by the lower left corner and try to resize it. => When the pager gets smaller, it quickly moves off screen. BAD => If you end the resize with the window off screen, it may be hard to retrieve it. BAD Summary: There are a lot of quirks and bugs. Is there any gain in usability to make up for this? > But in terms of configurability, I still think an option > where the user can decide to preserve the aspect ratio when resizing > the window or not allows the most freedom for users. But why would anybody want that? Does anybody really use a standalone pager, and if so, ever resizes it? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] Re: Idea: "elastic" move
On Mon, Nov 29, 2021 at 11:04:04PM +0100, Dominik Vogt wrote: > On Sun, Nov 21, 2021 at 03:29:29PM +0100, Dominik Vogt wrote: > > 1. When an interactive move starts: > > > >... > > - Warp the pointer to the middle of the screen. There it can > > always be moved in all directions. > > - If the pointer gets too close to the page edges, warp it back > > to the middle of the screen. > >... > > To get an idea how this feels, please try the branch > dv/elastic-paging-prototype. (It also contains a stack of > unrelated pending changes that can beignored). > > What's implemented: > > + Keeps the pointer away from the screen edges. The window can > be moved any distance (but is not visible off screen yet). > + The old paging code is removed. > > Not implemented: > > - Switching off the cursor. Now implemented. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] Re: Crashes with monitor_resolve_name
Updated patch in dv/monitor-parsing-fix. Doesn't take care of the NULL m->Desktops crash. Don't know how to reproduce. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: FvwmPager aspect ratio breaks FvwmButtons panel sliding
On Sat, Nov 27, 2021 at 02:44:07PM -0700, Jaimos Skriletz wrote: > On Sat, Nov 27, 2021 at 1:59 PM Dominik Vogt wrote: > > > > From commit 601c5c294a6a48fd402fbfca02b62142796167eb > > > > >* Improved use of sizehints to set both minimum window size, and to > > > preserve aspect ratio when resizing the pager. The aspect ratio is > > > set to the initial size of the window unless the user sets window > > > size with the Geometry option. > > > > x_pager.c: > > > + sizehints.flags = (sizehints.flags | PAspect); > > > + sizehints.min_aspect.x = sizehints.max_aspect.x = window_w; > > > + sizehints.min_aspect.y = sizehints.max_aspect.y = window_h; > > > > I'm against this change. > > > > 1) It breaks sliding the pager from a panel button. > > 2) fvwm's module were always supposed to deal with any size, even > > 1x1 or 65535x65535. > Not strictly true, there are some size increment hints that force the > pager's size to be an integer multiple of the number of pages shown, > so each page has the same number of pixels. So there were already some > restrictions on the sizes it could be. Right. The whole sizehints can be removed, except the width, height, usposition and gravity. All they do is impose unnecessary limitations: * Nobody cares if the width is not a multiple of the number of pages. If the pager is 299 pixels wide and there are three pages, then two of them have 100 pixels assigned and one has 99. So what? * Base size: If the pager gets unusable below a certain size, then do not make it smaller than that if you don't like it. I do resize modules to a very small size to get them out of the way sometimes (mostly FvwmButtons). Twenty years ago I personally removed all limitations on pager size. It seems I missed the standalone limitations because I only have one in the pager. FvwmPager is abolutely happy with any geometry (although it doesn't look good if it doesn't have at least two pixels per page). It still displays useful information even if it's only one pixel high. :-) > > 3) It makes it virtually impossible to have the pager fill the whole > > allocated area inside FvwmButtons. > > Can FvwmButtons be fixed to better deal with this? 1) No. Sliding breaks aspect ratio by design. 3) It's not FvwmButton's fault, it just honours the requersted aspect ratio because it has to. Programs might behave badly if their hints are ignored. Fvwm's modules are always fine if their hints are ignored because they are written to be fine. > > I see no benefit in these new limits. Nobody resizes the pager > > manually, and insde FvwmButtons it's really harmful. Can I remove > > this please? (See attached patch.) > > This can currently be disabled by proving an initial Geometry (though > it needs to be bigger than some minimum size I recall like 100x100) That's the value in the sizehints, but fvwm ignores that anyway. To get the initial size it looks at the real window size, not at the hints. In reality it's only 1 pixel per displayed page, and even that's unnecessary. The drawing and page management code can deal with pages that have not even a single pixel allocated to them. Let's remove the lower limit too. > then FvwmButtons will resize to fit without preserving aspect ratio. FvwmButtons does honour the aspect ratio (see above). > At the time we chose to have it be disabled via just setting a > geometry string vs having an additional option to turn preserving > aspect ratio on/off. I would rather have an option to allow users to > toggle this vs just disable it. I disagree. From experience, using the aspect ratio always annoys the user when resizing a window. It's fine for initial size calcualtions, but after that, leave it to the user. There are rare exceptions like video players. It also causes uncommon problems like with sliding, swallowing in FvwmButtons, or using "maximize grow grow" in a script. Imposing limitations on the use of fvwm's modules is not the fvwm way of doing things. FvwmPager is capable of dealing with any geometry, like all other modules. New patch attached -> no problem with 1x1 pixels even when the desktop is 50x50. Ciao Dominik ^_^ ^_^ -- Dominik Vogt From 565d878e776ca2cc21d89c1ff05a6c9e36f5cf57 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Sat, 27 Nov 2021 21:57:29 +0100 Subject: [PATCH] Remove size and resize limitation from FvwmPager. The limits are unnecessary, and the aspect ratio gives FvwmButtons a hard time to accomodate the pager when swallowed. --- modules/FvwmPager/x_pager.c | 42 +++-- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/modules/FvwmPager/x_pager.c b/modules/FvwmPager/x_pager.c index
FvwmPager aspect ratio breaks FvwmButtons panel sliding
From commit 601c5c294a6a48fd402fbfca02b62142796167eb
>* Improved use of sizehints to set both minimum window size, and to
> preserve aspect ratio when resizing the pager. The aspect ratio is
> set to the initial size of the window unless the user sets window
> size with the Geometry option.
x_pager.c:
> + sizehints.flags = (sizehints.flags | PAspect);
> + sizehints.min_aspect.x = sizehints.max_aspect.x = window_w;
> + sizehints.min_aspect.y = sizehints.max_aspect.y = window_h;
I'm against this change.
1) It breaks sliding the pager from a panel button.
2) fvwm's module were always supposed to deal with any size, even
1x1 or 65535x65535.
3) It makes it virtually impossible to have the pager fill the whole
allocated area inside FvwmButtons.
I see no benefit in these new limits. Nobody resizes the pager
manually, and insde FvwmButtons it's really harmful. Can I remove
this please? (See attached patch.)
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 72857bb298ac74707efbdd372fc83943218a3219 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 27 Nov 2021 21:57:29 +0100
Subject: [PATCH] Remove aspect ratio from pager.
---
modules/FvwmPager/x_pager.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/modules/FvwmPager/x_pager.c b/modules/FvwmPager/x_pager.c
index 4718cd56..ab806d1a 100644
--- a/modules/FvwmPager/x_pager.c
+++ b/modules/FvwmPager/x_pager.c
@@ -122,7 +122,7 @@ Window icon_win; /* icon window */
static int MyVx, MyVy; /* copy of Scr.Vx/y for drag logic */
-static void adjust_for_sizehints(int, int, bool);
+static void adjust_for_sizehints(int, int);
static rectangle CalcGeom(PagerWindow *, bool);
static rectangle set_vp_size_and_loc(void);
static void fvwmrec_to_pager(rectangle *, bool);
@@ -446,7 +446,7 @@ void initialize_balloon_window(void)
* should get added to a clean up TODO at some point.
*/
static void
-adjust_for_sizehints(int VxPages, int VyPages, bool check_aspect)
+adjust_for_sizehints(int VxPages, int VyPages)
{
/* Resize increments are one pixel per visible page. */
sizehints.width_inc = Columns * VxPages;
@@ -470,10 +470,6 @@ adjust_for_sizehints(int VxPages, int VyPages, bool check_aspect)
pwindow.height = pwindow.height * sizehints.height_inc +
sizehints.base_height;
}
- if (check_aspect && sizehints.min_aspect.x > 0) {
- sizehints.min_aspect.x = sizehints.max_aspect.x = pwindow.width;
- sizehints.min_aspect.y = sizehints.max_aspect.y = pwindow.height;
- }
desk_w = (pwindow.width - Columns + 1) / Columns;
desk_h = (pwindow.height - Rows * label_h - Rows + 1) / Rows;
@@ -714,14 +710,11 @@ void initialize_pager(void)
pwindow.height = (VyPages * vHeight * Rows) / Scr.VScale +
label_h * Rows + Rows;
}
- sizehints.flags = (sizehints.flags | PAspect);
- sizehints.min_aspect.x = sizehints.max_aspect.x = pwindow.width;
- sizehints.min_aspect.y = sizehints.max_aspect.y = pwindow.height;
}
/* Adjust the window to handle these new sizehints. This is also called
* from ReConfigure().
*/
- adjust_for_sizehints(VxPages, VyPages, true);
+ adjust_for_sizehints(VxPages, VyPages);
if (is_transient)
{
@@ -1462,7 +1455,7 @@ void ReConfigure(void)
}
is_size_changed = (old_ww != pwindow.width || old_wh != pwindow.height);
- adjust_for_sizehints(VxPages, VyPages, false);
+ adjust_for_sizehints(VxPages, VyPages);
XSetWMNormalHints(dpy,Scr.Pager_w,);
--
2.30.2
Re: [PATCH] Re: Crashes with monitor_resolve_name
On Sat, Nov 27, 2021 at 08:30:52PM +0100, Dominik Vogt wrote:
> On Sat, Nov 27, 2021 at 07:02:29PM +0100, Dominik Vogt wrote:
> > There are some NULL pointer crashes and bugs telated to
> > moitor_resove_ame():
>
> Attempt to fix these, please proof read the patch.
>
> * Parsing fixes.
> * monitor_resolve_name() does not crash if scr == NULL but returns
>NULL.
> * Callers deal with NULL beng returned.
> * FScreenGetScrRect() uses the global screen f the screen is not
>found.
> * Export monitor_global. (A functions seems to be overkill.)
> * Some other minor related cleanup.
>
> Doesn't crash, but I can't test it with a single monitor. One
> thing to double check if whether callers should use the global,
> primary or current screen if monitor_resolve_name() returns NULL.
> My guesses may not be all correct.
Updated patch attached.
During testing I got a crash in ewmh.c:1049 because m->Desktops of
the primary screen is NULL:
if (
m->Desktops->ewmh_working_area.x != x ||
m->Desktops->ewmh_working_area.y != y ||
m->Desktops->ewmh_working_area.width != width ||
m->Desktops->ewmh_working_area.height != height)
{
I think that's unrelated to the patch. It was triggered by
destroying an FvwmConsole window.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x55ef2959458c in ewmh_ComputeAndSetWorkArea (m=m@entry=0x55ef2b101a20)
at ewmh.c:1048
1048if (
(gdb) bt
#0 0x55ef2959458c in ewmh_ComputeAndSetWorkArea (m=m@entry=0x55ef2b101a20)
at ewmh.c:1048
#1 0x55ef29594ebe in EWMH_WindowDestroyed () at ewmh.c:1863
#2 0x55ef29565137 in HandleUnmapNotify (ea=)
at events.c:3903
#3 0x55ef29563d3e in dispatch_event (e=e@entry=0x7ffef344f560)
at events.c:4185
#4 0x55ef29564b0a in HandleEvents () at events.c:4231
#5 0x55ef2958304f in main (argc=, argv=)
at fvwm3.c:2547
Don't know how I triggered that. I was testing
GotoDesk/Page/DeskAndPage with that console.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 7e8d87d872b571c0b7d5c1fbc024c4a8339cf1e7 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 27 Nov 2021 18:01:29 +0100
Subject: [PATCH] Fix monitor parsing II.
---
fvwm/ewmh_conf.c | 9 +--
fvwm/expand.c| 4 +-
fvwm/move_resize.c | 14 ++---
fvwm/placement.c | 8 ++-
fvwm/virtual.c | 100 ++-
libs/FScreen.c | 83 -
libs/FScreen.h | 1 +
modules/FvwmIconMan/readconfig.c | 4 ++
8 files changed, 95 insertions(+), 128 deletions(-)
diff --git a/fvwm/ewmh_conf.c b/fvwm/ewmh_conf.c
index cb2bb126..e94604f0 100644
--- a/fvwm/ewmh_conf.c
+++ b/fvwm/ewmh_conf.c
@@ -110,8 +110,7 @@ void CMD_EwmhNumberOfDesktops(F_CMD_ARGS)
option = PeekToken(action, );
if ((m = monitor_resolve_name(option)) == NULL) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
+ fvwm_debug(__func__, "Invalid screen: %s", option);
}
}
@@ -150,10 +149,8 @@ void CMD_EwmhBaseStruts(F_CMD_ARGS)
/* Actually get the screen value. */
option = PeekToken(action, );
- m = monitor_resolve_name(option);
- if (strcmp(m->si->name, option) != 0) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
+ if ((m = monitor_resolve_name(option)) == NULL) {
+ fvwm_debug(__func__, "Invalid screen: %s", option);
return;
}
}
diff --git a/fvwm/expand.c b/fvwm/expand.c
index 44115ade..7aee8790 100644
--- a/fvwm/expand.c
+++ b/fvwm/expand.c
@@ -538,9 +538,7 @@ static signed int expand_vars_extended(
rest_s = fxstrdup(rest);
while ((m_name = strsep(_s, ".")) != NULL) {
mon2 = monitor_resolve_name(m_name);
- if (m_name == NULL)
-return -1;
- if (strcmp(mon2->si->name, m_name) == 1)
+ if (mon2 == NULL)
return -1;
/* Skip over the monitor name. */
diff --git a/fvwm/move_resize.c b/fvwm/move_resize.c
index cd070fa8..47fd322e 100644
--- a/fvwm/move_resize.c
+++ b/fvwm/move_resize.c
@@ -2071,11 +2071,15 @@ static void __move_window(F_CMD_ARGS, Bool do_animate, int mode)
rectangle r;
rectangle s;
rectangle t;
- struct monitor *m = monitor_get_current();
+ struct monitor *m;
char *token;
- if (action != NULL && (token = PeekToken(action, )) != NULL)
- m = monitor_resolve_name(token);
+ token = PeekToken(action, );
+ m = monitor_resolve_name(token);
+ if (m == NULL)
+ {
+ m = monitor_get_current();
+ }
s.x = m->si->x;
s.y = m->si->y;
@@ -3301,10 +3305,6 @@ void CMD_GeometryWindow(F_CMD_ARGS)
if (token != NULL)
{
Scr.SizeWindow.m = monitor_resolve_name(token);
-if (strcasecmp(Scr.SizeWindow.m->si->name, token) != 0)
[PATCH] Re: Crashes with monitor_resolve_name
On Sat, Nov 27, 2021 at 07:02:29PM +0100, Dominik Vogt wrote:
> There are some NULL pointer crashes and bugs telated to
> moitor_resove_ame():
Attempt to fix these, please proof read the patch.
* Parsing fixes.
* monitor_resolve_name() does not crash if scr == NULL but returns
NULL.
* Callers deal with NULL beng returned.
* FScreenGetScrRect() uses the global screen f the screen is not
found.
* Export monitor_global. (A functions seems to be overkill.)
* Some other minor related cleanup.
Doesn't crash, but I can't test it with a single monitor. One
thing to double check if whether callers should use the global,
primary or current screen if monitor_resolve_name() returns NULL.
My guesses may not be all correct.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 288bfd7f95a87bdeb5783d5d4a7afcd1926680d0 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 27 Nov 2021 18:01:29 +0100
Subject: [PATCH] Fix monitor parsing.
---
fvwm/ewmh_conf.c | 9 +--
fvwm/expand.c| 4 +-
fvwm/move_resize.c | 14 ++---
fvwm/placement.c | 8 ++-
fvwm/virtual.c | 100 ++-
libs/FScreen.c | 37 ++--
libs/FScreen.h | 1 +
modules/FvwmIconMan/readconfig.c | 4 ++
8 files changed, 87 insertions(+), 90 deletions(-)
diff --git a/fvwm/ewmh_conf.c b/fvwm/ewmh_conf.c
index cb2bb126..e94604f0 100644
--- a/fvwm/ewmh_conf.c
+++ b/fvwm/ewmh_conf.c
@@ -110,8 +110,7 @@ void CMD_EwmhNumberOfDesktops(F_CMD_ARGS)
option = PeekToken(action, );
if ((m = monitor_resolve_name(option)) == NULL) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
+ fvwm_debug(__func__, "Invalid screen: %s", option);
}
}
@@ -150,10 +149,8 @@ void CMD_EwmhBaseStruts(F_CMD_ARGS)
/* Actually get the screen value. */
option = PeekToken(action, );
- m = monitor_resolve_name(option);
- if (strcmp(m->si->name, option) != 0) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
+ if ((m = monitor_resolve_name(option)) == NULL) {
+ fvwm_debug(__func__, "Invalid screen: %s", option);
return;
}
}
diff --git a/fvwm/expand.c b/fvwm/expand.c
index 44115ade..7aee8790 100644
--- a/fvwm/expand.c
+++ b/fvwm/expand.c
@@ -538,9 +538,7 @@ static signed int expand_vars_extended(
rest_s = fxstrdup(rest);
while ((m_name = strsep(_s, ".")) != NULL) {
mon2 = monitor_resolve_name(m_name);
- if (m_name == NULL)
-return -1;
- if (strcmp(mon2->si->name, m_name) == 1)
+ if (mon2 == NULL)
return -1;
/* Skip over the monitor name. */
diff --git a/fvwm/move_resize.c b/fvwm/move_resize.c
index cd070fa8..47fd322e 100644
--- a/fvwm/move_resize.c
+++ b/fvwm/move_resize.c
@@ -2071,11 +2071,15 @@ static void __move_window(F_CMD_ARGS, Bool do_animate, int mode)
rectangle r;
rectangle s;
rectangle t;
- struct monitor *m = monitor_get_current();
+ struct monitor *m;
char *token;
- if (action != NULL && (token = PeekToken(action, )) != NULL)
- m = monitor_resolve_name(token);
+ token = PeekToken(action, );
+ m = monitor_resolve_name(token);
+ if (m == NULL)
+ {
+ m = monitor_get_current();
+ }
s.x = m->si->x;
s.y = m->si->y;
@@ -3301,10 +3305,6 @@ void CMD_GeometryWindow(F_CMD_ARGS)
if (token != NULL)
{
Scr.SizeWindow.m = monitor_resolve_name(token);
-if (strcasecmp(Scr.SizeWindow.m->si->name, token) != 0) {
- /* Incorrect RandR screen found. */
- Scr.SizeWindow.m = NULL;
-}
}
}
}
diff --git a/fvwm/placement.c b/fvwm/placement.c
index 9d081d98..0169730c 100644
--- a/fvwm/placement.c
+++ b/fvwm/placement.c
@@ -1732,6 +1732,8 @@ static int __place_window(
if (flags.do_honor_starts_on_screen)
{
fscreen_scr_arg arg;
+ struct monitor *m;
+
arg.mouse_ev = NULL;
/* FIXME: expand the screen name here. It's possible
@@ -1762,8 +1764,12 @@ static int __place_window(
* "_global" screen, which is a faked monitor for the
* purposes of an older API.
*/
+ m = NULL;
if (strcmp(arg.name, "g") != 0)
-fw->m = monitor_resolve_name(arg.name);
+m = monitor_resolve_name(arg.name);
+ if (m == NULL)
+m = monitor_get_current();
+ fw->m = m;
free(e);
}
else
diff --git a/fvwm/virtual.c b/fvwm/virtual.c
index dacb39cb..8cbcc712 100644
--- a/fvwm/virtual.c
+++ b/fvwm/virtual.c
@@ -1783,41 +1783,38 @@ void do_move_window_to_desk(FvwmWindow *fw, int desk)
return;
}
-Bool get_page_arguments(FvwmWindow *fw, char *action, int *page_x, int *page_y, struct monitor **mret)
+Bool get_page_arguments(
+ FvwmWindow *fw, char *action, int *page_x, int *page_y,
+ struct monitor **mret)
{
int val[2];
int suffix[2];
int mw, mh;
char *token;
- char *taction, *action_cpy, *action_cpy_start;
+ char *taction;
+ ch
Crashes with monitor_resolve_name
There are some NULL pointer crashes and bugs telated to
moitor_resove_ame():
1. The fuction assumes that it's always called with a non-null
"scr" pointer:
monitor_resolve_name(const char *scr)
{
...
if (strcmp(scr, "g") == 0) {
This is not the case. Several places in the source call the
function without validating that the pointer in non-NULL.
This can be triggered with the command "desk" (no arguments).
2. #1 is because bugs in the parsing code that may be the result
of a misunderstanding how the parsing functions work:
-- snip --
01 void CMD_GotoDesk(F_CMD_ARGS)
02 {
03 ...
04 action_cpy = strdup(action);
05 action_cpy_start = action_cpy;
06 token = PeekToken(action_cpy, _cpy);
07
08 m = monitor_resolve_name(token);
09 if (strcmp(m->si->name, token) != 0)
10 m = m_use;
11 else
12 PeekToken(action, );
13
14 new_desk = GetDeskNumber(m, action, m->virtual_scr.CurrentDesk);
-- snip --
Problems here:
line 02 + 03: This is pointless. Neither PeekToken nor the other
parsing functions modify the "action".
line 04: fxstrdup() should be used.
line 06: This seems to assume that PeekToken always returns a
string pointer. This is *not* the case! The parsing
functions never return an empty string.
Also, if the caller is not interested in the rest of the
command line, just pass NULL as second argument:
token = PeekToken(action, NULL);
line 08: Crashes because of #1.
line 09: Crashes because token can be NULL.
line 12: The proper way to skip tokens at the beginning of a
string is
action = SkipNTokens(action, 1);
line 14: OK because GetDeskNumber uses MatchToken, which is aware
of all possible NULL pointers.
--
Also, there's a real parsing bug:
If called with "desk arg1 arg2":
Case 1: arg1 is a monitor name.
-> calls GetDeskNumberGetDeskNumber with action = "arg1 arg2"
^^^
Case 2: arg1 is no monitor name.
-> calls GetDeskNumberGetDeskNumber with action = "arg2"
^^
That can't be correct.
--
(Specifying a monitor name is not documented in the man page).
--
Are there other places with unusual monitor name parsing?
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Re: Flocale leak
On Fri, Nov 26, 2021 at 01:03:40PM +0100, Dominik Vogt wrote:
> Stuck with this one, ideas welcome:
>
> On Wed, Nov 24, 2021 at 03:22:54PM +0100, Dominik Vogt wrote:
> > ==8482== 38 (16 direct, 22 indirect) bytes in 1 blocks are definitely lost
> > in loss record 184 of 536
> > ==8482==at 0x48386AF: malloc (in
> > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==8482==by 0x483ADE7: realloc (in
> > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==8482==by 0x4CE950B: XCreateFontSet (in
> > /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0)
> > ==8482==by 0x18EB64: FlocaleGetFontSet (Flocale.c:1152)
> > ==8482==by 0x18FD81: FlocaleGetFontOrFontSet (Flocale.c:1302)
> > ==8482==by 0x18FD81: FlocaleLoadFont (Flocale.c:1479)
> > ==8482==by 0x14A314: CMD_DefaultFont (builtins.c:3160)
It seems this is a bug in libx11-1.7.2. The attached simple test
program also has the leak.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
#include
#include
#include
#include
#define FN "7x13"
int main(int argc, char **argv)
{
Display *dpy;
int i;
char *locale;
locale = setlocale(LC_CTYPE, "");
assert(locale);
dpy = XOpenDisplay(":0");
assert(dpy);
for (i = 0; i < 10; i++)
{
XFontSet fs;
char **ml;
int mc;
fs = XCreateFontSet(dpy, FN, , , NULL);
if (fs)
{
if (ml)
XFreeStringList(ml);
XFreeFontSet(dpy, fs);
}
}
exit(0);
}
Re: Flocale leak
Stuck with this one, ideas welcome:
On Wed, Nov 24, 2021 at 03:22:54PM +0100, Dominik Vogt wrote:
> ==8482== 38 (16 direct, 22 indirect) bytes in 1 blocks are definitely lost in
> loss record 184 of 536
> ==8482==at 0x48386AF: malloc (in
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8482==by 0x483ADE7: realloc (in
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8482==by 0x4CE950B: XCreateFontSet (in
> /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0)
> ==8482==by 0x18EB64: FlocaleGetFontSet (Flocale.c:1152)
> ==8482==by 0x18FD81: FlocaleGetFontOrFontSet (Flocale.c:1302)
> ==8482==by 0x18FD81: FlocaleLoadFont (Flocale.c:1479)
> ==8482==by 0x14A314: CMD_DefaultFont (builtins.c:3160)
This simple reduced test case still leaks memory:
char **ml;
int mc;
char *ds;
fontset = XCreateFontSet(dpy, fn_fixed, , , );
if (fontset)
{
if (ml) XFreeStringList(ml);
XFreeFontSet(dpy, fontset);
}
XFreeFontSet seems to not release all resources allocated by
XCreateFontSet. It's not a one time leak; each Create/Free pair
leaks more memory. It's not the "ml" bit which is always NULL.
ds always returns the same pointer, and it's not supposed to be
freed.
There's nothing in the net about memory leaks in XFreeFontSet.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Window title leak
This is a tough one: ==14237== 11 bytes in 1 blocks are definitely lost in loss record 11 of 102 ==14237==at 0x483877F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==14237==by 0x5FF7E4A: strdup (strdup.c:42) ==14237==by 0x14B75A: interpolate_titleformat_name (add_window.c:678) ==14237==by 0x14C27A: setup_visible_names (add_window.c:1784) ==14237==by 0x14E714: AddWindow (add_window.c:2417) ==14237==by 0x136A4E: HandleMapRequestKeepRaised (events.c:3068) ==14237==by 0x150210: CaptureAllWindows (add_window.c:3775) ==14237==by 0x1560F8: StartupStuff (fvwm3.c:1488) ==14237==by 0x1386F3: My_XNextEvent (events.c:4289) ==14237==by 0x139C1C: HandleEvents (events.c:4222) ==14237==by 0x158C4D: main (fvwm3.c:2547) To reproduce: * Start fvwm with valgrind on a Xephyr screen * Kill everythign except an rxvt (or whatever) * Open FvwmConsole and type style rxvt startsonpage 0 0 destroystyle rxvt (Any style option causes this.) * Quit fvwm So, a title is allocated and leaked. It must have to do with the style update procedure. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: UPDATE_FVWM_SCREEN macro?
On Thu, Nov 25, 2021 at 10:56:19PM +, Thomas Adam wrote: > On Wed, Nov 24, 2021 at 03:43:47PM +0100, Dominik Vogt wrote: > > Does anybody know why this is a macro and not a function? > > (screen.h) > > Because when I wrote it, it wasn't as complex as it is now. > > See the ta/update-fvwm-screen branch, I've converted it to a function instead. Looks good, okay to apply. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: memleak in FScreenParseGeometryWithScreen
On Thu, Nov 25, 2021 at 11:04:25PM +, Thomas Adam wrote: > On Wed, Nov 24, 2021 at 03:13:34PM +0100, Dominik Vogt wrote: > > From FScreen.c:FScreenParseGeometryWithScreen(): > > > > I can't fix this because I don't understand what the code does. > > I will fix this. Too late, but please do look at the two commits I made. The structure "icon_boxes" (fvwm.h) really should not contain an allocated string. Can we not store the numeric screen rectangle instead of the name? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Transforming program names
Recently there was the question wether the "transform" logic aon man pages was still necessary. It seems not. --program-transform="s/fvwm/xyz/;s/Fvwm/Xyz/" Renames all man pages too: fvwm*.1 -> xyz*.1 Fvwm*.1 -> Xyz*.1 (The module names are not transformed, but that's a different topic.) It seems there is no need to change anything. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
UPDATE_FVWM_SCREEN macro?
Does anybody know why this is a macro and not a function?
(screen.h)
#define UPDATE_FVWM_SCREEN(fw) \
do { \
rectangle g; \
struct monitor *mnew; \
\
get_unshaded_geometry((fw), ); \
mnew = FindScreenOfXY((fw)->g.frame.x, (fw)->g.frame.y); \
/* Avoid unnecessary updates. */ \
if (mnew == (fw)->m) \
break; \
(fw)->m_prev = (fw)->m;\
(fw)->m = mnew;\
(fw)->Desk = mnew->virtual_scr.CurrentDesk;\
EWMH_SetCurrentDesktop((fw)->m); \
desk_add_fw((fw)); \
BroadcastConfig(M_CONFIGURE_WINDOW, (fw)); \
} while(0)
The "do { ... } while (0)" was probably written by me.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Flocale leak
I can't figure out when this memory is leaked: ==8482== 21 bytes in 1 blocks are definitely lost in loss record 155 of 536 ==8482==at 0x483877F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==8482==by 0x5FE76A7: __vasprintf_internal (vasprintf.c:71) ==8482==by 0x1AC2A6: xvasprintf (safemalloc.c:104) ==8482==by 0x1AC36A: xasprintf (safemalloc.c:93) ==8482==by 0x1A5990: translit_csname (Ficonv.c:93) ==8482==by 0x1A5DB8: set_iconv_charset_index (Ficonv.c:203) ==8482==by 0x1A5DB8: FiconvSetupConversion (Ficonv.c:437) ==8482==by 0x1A6134: FiconvCharsetToUtf8 (Ficonv.c:548) ==8482==by 0x18F10E: FlocaleEncodeString (Flocale.c:464) ==8482==by 0x19086D: FlocaleTextWidth (Flocale.c:2192) ==8482==by 0x13E299: resize_geometry_window (move_resize.c:1123) ==8482==by 0x158DD6: main (fvwm3.c:2543) The memory is allocated through Ficonv.c:203: FLC_SET_ICONV_TRANSLIT_CHARSET( fc, fxstrdup( translit_csname( FLC_GET_LOCALE_CHARSET( fc,i; This stores it in fc->translit_csname. I've no idea where it's lost. I'd assume that charsets are not ever freed or lost. -- Two more leaks that need to be checked: ==8482== 38 (16 direct, 22 indirect) bytes in 1 blocks are definitely lost in loss record 184 of 536 ==8482==at 0x48386AF: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==8482==by 0x483ADE7: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==8482==by 0x4CE950B: XCreateFontSet (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) ==8482==by 0x18EB64: FlocaleGetFontSet (Flocale.c:1152) ==8482==by 0x18FD81: FlocaleGetFontOrFontSet (Flocale.c:1302) ==8482==by 0x18FD81: FlocaleLoadFont (Flocale.c:1479) ==8482==by 0x14A314: CMD_DefaultFont (builtins.c:3160) ==8482==by 0x1740B4: __execute_command_line (functions.c:669) ==8482==by 0x174CBA: execute_function (functions.c:1240) ==8482==by 0x18B3F2: run_command_stream (read.c:148) ==8482==by 0x18B53B: run_command_file (read.c:256) ==8482==by 0x15903F: main (fvwm3.c:2494) ==8482== ==8482== 38 (16 direct, 22 indirect) bytes in 1 blocks are definitely lost in loss record 185 of 536 ==8482==at 0x48386AF: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==8482==by 0x483ADE7: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==8482==by 0x4CE950B: XCreateFontSet (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0) ==8482==by 0x18EB64: FlocaleGetFontSet (Flocale.c:1152) ==8482==by 0x18FD81: FlocaleGetFontOrFontSet (Flocale.c:1302) ==8482==by 0x18FD81: FlocaleLoadFont (Flocale.c:1479) ==8482==by 0x14C721: setup_icon_font (add_window.c:1946) ==8482==by 0x14E89F: AddWindow (add_window.c:2414) ==8482==by 0x13698C: HandleMapRequestKeepRaised (events.c:3068) ==8482==by 0x136DC3: HandleMapRequest (events.c:3005) ==8482==by 0x138D50: dispatch_event (events.c:4186) ==8482==by 0x139B1C: HandleEvents (events.c:4232) ==8482==by 0x158DE5: main (fvwm3.c:2547) Ciao Dominik ^_^ ^_^ -- Dominik Vogt
memleak in FScreenParseGeometryWithScreen
From FScreen.c:FScreenParseGeometryWithScreen(): 943 copy = fxstrdup(parsestring); 944 copy = strsep(, "@"); 945 946 *screen_return = fxstrdup(parsestring); 947 geom_str = strsep(, "@"); 948 copy = geom_str; I can't fix this because I don't understand what the code does. Line 943 allocates some memory and stores the pointer in "copy.OK Line 944 throws away the just allocated "copy" LEAK A It tokenizes the "parsestring" passed in by the caller, thus destroying its value. BUG The code seems to assume that after "copy = strsep()" the copy pointer would point to the _next_ token? But it will be identical to "parsestring". BUG? Line 946 Returns a copy of the token = the geometry string without the screen portion through *screen_return. There are two bugs here: 1) The callers expect the token with the screen string, not the token with the geometry string. BUG 2) The callers don't know that they have to free() the returned pointer. LEAK B Line 947 tokenizes the already tokenized token again, which BUG is a no-op. Line 948 overwrites "copy" with its own value => no-op. BUG -- Summary: 1) LEAK A is inside the function and can be fixed there. 2) LEAK B is in the callers. None of the callers I checked is aware that *screen_return has to be free()'d. 3) *screen_return contains the geometry portion of the string, but callers expect it to contain the screen portion. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Commit 3f61e78 breaks fvwm3 -v
On Wed, Nov 24, 2021 at 03:49:19AM -0700, Jaimos Skriletz wrote: > Hello, > > On my system the following commit makes it so fvwm3 -v fails to start > on my system. But fvwm3 works just fine. I tracked the issue to this > commit, as this is the first commit in which fvwm3 -v no longer works > when running startx using a ~/.xsession file to launch fvwm3. > > https://github.com/fvwmorg/fvwm3/commit/3f61e78201a452a69c74ccde036e371391e0dfd3 > So unsure if fvwm3 is crashing or just exiting for some reason. Crashing because it used the not yet set log_file_name. It was really difficult to rewrite the logic, and the tests missed that case. Fixed on master. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Committed Xinerama cleanup and a restructuring patch
Just committed 1) The Xinerama cleanup. 2) A big patch that makes move_resize.c and some related places in other files use the types position, size_rect and rectangle. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH/RFC] Xinerama
On Tue, Nov 23, 2021 at 01:44:52AM +0100, Dominik Vogt wrote: > Menu foomenu monitor c c > > Should place the menu in the middle of the current monitor. Or Menu foomenu c-20 c+30 should place it 20% of the monitor size left from the center and 30% below the center. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH/RFC] Xinerama
On Mon, Nov 22, 2021 at 11:52:38PM +, Thomas Adam wrote: > On Tue, Nov 23, 2021 at 12:31:15AM +0100, Dominik Vogt wrote: > > The old manmage said: > > > > XineramaRoot > > > > the root window of the whole Xinerama screen. Equivalent to > > "root" when Xinerama is not used. > > > > Can you please restate that for me? What is "whole Xinerama > > screen" in xrandr terms? > > >From a quick scan of the code in menus.c where xineramaroot is being used, > it's not entirely clear to me, In the past it referenced the current Xinerama screen, not the whole root window. When a menu is placed the code needs a reference rectangle to calculate the menu position. It can be relative to the pointer, to a window, to the root window, to a monitor etc. > but if this option is meant to do what I think > it is, then this would be taking the width/height of the monitor the > pointer/window is on, rather than referencing the width/height of the entire > root window. That would be correct. Renamed it to "monitor" and "fMonitorContext". There's only one place in the code that evaluates the flag, and that looks fishy. Can you test it? Menu foomenu monitor c c Should place the menu in the middle of the current monitor. The code is on the dv/devel branch (you may not want the top commit on that branch). Ciao Dominik ^_^ ^_^ -- Dominik Vogt
NEW_PARSER.md
The file now contains an overview of the current parser implementation. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH/RFC] Xinerama
On Mon, Nov 22, 2021 at 10:46:20PM +, Thomas Adam wrote:
> On Mon, Nov 22, 2021 at 11:26:53PM +0100, Dominik Vogt wrote:
> > On Sun, Nov 21, 2021 at 12:39:07AM +0100, Dominik Vogt wrote:
> > > Patch attached. Can you please double check and fill in the gaps?
> > > (see comments belo).
> > >
> > > > > ./fvwm/menus.c: fXineramaRoot = False;
> > > > > ./fvwm/menus.c: else if (StrEquals(tok,"xineramaroot"))
> > >
> > >
> > > Not sure what the right name of the option and the flag variable
> > > should be. There's only a placeholder in the man page either.
>
> Making that change would be syntactically breaking people's configs, but... I
> bet it's a rarely-used option that should just be "screenroot" or something.
It can have two names, a real one and still "xineramaroot" for
compatibility. Screenroot? Rootscreen? Just tell me a good
name.
The old manmage said:
XineramaRoot
the root window of the whole Xinerama screen. Equivalent to
"root" when Xinerama is not used.
Can you please restate that for me? What is "whole Xinerama
screen" in xrandr terms?
> > > > > ./perllib/FVWM/Commands.pm: descr => q{Move a window to
> > > > > another Xinerama screen},
> > >
> > > [MOVE_TO_SCREEN]
> > > This one should be just renamed to just "screen"?
>
> Yes please.
Good. It's already part of the patch.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Re: [PATCH/RFC] Xinerama
On Sun, Nov 21, 2021 at 12:39:07AM +0100, Dominik Vogt wrote:
> Patch attached. Can you please double check and fill in the gaps?
> (see comments belo).
>
> > > ./fvwm/menus.c: fXineramaRoot = False;
> > > ./fvwm/menus.c: else if (StrEquals(tok,"xineramaroot"))
>
>
> Not sure what the right name of the option and the flag variable
> should be. There's only a placeholder in the man page either.
>
> > > ./perllib/FVWM/Commands.pm: descr => q{Move a window to
> > > another Xinerama screen},
>
> [MOVE_TO_SCREEN]
> This one should be just renamed to just "screen"?
Ping.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
Re: [RFC] parser branches ready for commit
On Mon, Nov 22, 2021 at 06:16:02PM +, Thomas Adam wrote: > On Mon, Nov 22, 2021 at 12:02:33AM +0100, Dominik Vogt wrote: > > Mostly done, except a couple of comments where there's still work > > to do. > > OK, I'll wait for those. These won't go away anytime soon. First I'd need to figure out what their mening and context is. :-) > I've not noticed any crashes from running this for three days now. > > Note that I still don't think we should hide the debugging behind > '#define OCP_DEBUG 1' What do you mean with "hide"? > even though it's always set to on at compile time. > Certainly not for new code. > We could make it a BugOpts option if it really > mattered. I'm not saying we need to fix this now, mind you. I want to remove completely it once the code has been tested well enough. It's just there for the moment in case something happens. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Idea: "elastic" move
On Sun, Nov 21, 2021 at 03:29:29PM +0100, Dominik Vogt wrote:
> - If the viewport jumps use some kind of smooth animation.
> This may be especially important if the viewport snaps to a
> full page when the move ends.
Tried this with the attached patch. It's not smooth and it
doesn't look good. Windows that move into view are only half
drawn because the screen is grabbed, and the pager flickers.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 9963bda7bf0fda6f6a3f4750b7e7e5911a04066a Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Mon, 22 Nov 2021 01:37:04 +0100
Subject: [PATCH] !!! MoveViewport animation
---
fvwm/virtual.c | 357 +++--
1 file changed, 198 insertions(+), 159 deletions(-)
diff --git a/fvwm/virtual.c b/fvwm/virtual.c
index 94ad4418..e965322b 100644
--- a/fvwm/virtual.c
+++ b/fvwm/virtual.c
@@ -1393,14 +1393,182 @@ Bool is_pan_frame(Window w)
* Moves the viewport within the virtual desktop
*
*/
-void MoveViewport(struct monitor *m, int newx, int newy, Bool grab)
+static void move_viewport_delta(
+ struct monitor *m, position delta, position page_tl, position page_br,
+ int do_broadcast)
{
- struct monitor *m_loop;
+ struct monitor *mloop;
FvwmWindow *t, *t1;
- int deltax,deltay;
- int PageTop, PageLeft;
- int PageBottom, PageRight;
int txl, txr, tyt, tyb;
+
+ if (do_broadcast)
+ {
+ TAILQ_FOREACH(mloop, _q, entry)
+ {
+ BroadcastPacket(
+M_NEW_PAGE, 8,
+(long)mloop->virtual_scr.Vx,
+(long)mloop->virtual_scr.Vy,
+(long)mloop->virtual_scr.CurrentDesk,
+(long) monitor_get_all_widths(),
+(long) monitor_get_all_heights(),
+(long)((mloop->virtual_scr.VxMax /
+ monitor_get_all_widths()) + 1),
+(long)((mloop->virtual_scr.VyMax /
+ monitor_get_all_heights()) + 1),
+(long)mloop->si->rr_output);
+ }
+ }
+ /*
+ * RBW - 11/13/1998 - new: chase the chain
+ * bidirectionally, all at once! The idea is to move the
+ * windows that are moving out of the viewport from the bottom
+ * of the stacking order up, to minimize the expose-redraw
+ * overhead. Windows that will be moving into view will be
+ * moved top down, for the same reason. Use the new
+ * stacking-order chain, rather than the old last-focused
+ * chain.
+ *
+ * domivogt (29-Nov-1999): It's faster to first map windows
+ * top to bottom and then unmap windows bottom up.
+ */
+ /* TA: 2020-01-21: This change of skipping monitors will
+ * break using 'Scroll' and __drag_viewport(). We need to
+ * ensure we handle this case properly.
+ */
+ t = get_next_window_in_stack_ring();
+ while (t != )
+ {
+ if (NOT_GLOBALLY_ACTIVE(t->m, m)) {
+ /* Bump to next win... */
+ t = get_next_window_in_stack_ring(t);
+ continue;
+ }
+ /*
+ * If the window is moving into the viewport...
+ */
+ txl = t->g.frame.x;
+ tyt = t->g.frame.y;
+ txr = t->g.frame.x + t->g.frame.width - 1;
+ tyb = t->g.frame.y + t->g.frame.height - 1;
+ if (is_window_sticky_across_pages(t) &&
+ !IS_VIEWPORT_MOVED(t))
+ {
+ /* the absolute position has changed */
+ t->g.normal.x -= delta.x;
+ t->g.normal.y -= delta.y;
+ t->g.max.x -= delta.x;
+ t->g.max.y -= delta.y;
+ /* Block double move. */
+ SET_VIEWPORT_MOVED(t, 1);
+ }
+ if ((txr >= page_tl.x && txl <= page_br.x
+ && tyb >= page_tl.y && tyt <= page_br.y)
+ && !IS_VIEWPORT_MOVED(t)
+ && !IS_WINDOW_BEING_MOVED_OPAQUE(t))
+ {
+ /* Block double move. */
+ SET_VIEWPORT_MOVED(t, 1);
+ /* If the window is iconified, and sticky Icons is set,
+ * then the window should essentially be sticky */
+ if (!is_window_sticky_across_pages(t))
+ {
+if (IS_ICONIFIED(t))
+{
+ modify_icon_position(
+ t, delta.x, delta.y);
+ move_icon_to_position(t);
+ if (do_broadcast)
+ {
+ broadcast_icon_geometry(
+ t, False);
+ }
+}
+frame_setup_window(
+ t, t->g.frame.x + delta.x,
+ t->g.frame.y + delta.y,
+ t->g.frame.width,
+ t->g.frame.height, False);
+ }
+ }
+ /* Bump to next win... */
+ t = get_next_window_in_stack_ring(t);
+ }
+ t1 = get_prev_window_in_stack_ring();
+ while (t1 != )
+ {
+ if (NOT_GLOBALLY_ACTIVE(t1->m, m)) {
+ /* Bump to next win... */
+ t1 = get_prev_window_in_stack_ring(t1);
+ continue;
+ }
+ /*
+ *If the window is not moving into the viewport...
+ */
+ SET_VIEWPORT_MOVED(t, 1);
+ txl = t1->g.frame.x;
+ tyt = t1->g.frame.y;
+ txr = t1->g.frame.x + t1->g.frame.width - 1;
+ tyb = t1->g.frame.y + t1->g.frame.height - 1;
+ if (! (txr >= page_tl.x && txl <= page_br.x
+ && tyb >= page_tl.y && tyt <= page_br.y)
+ && !IS_VIEWPORT_MOVED(t1)
+ && !IS_WINDOW_BEING_MOVED_OPAQUE(t1))
+ {
+ /* If the window is iconified, an
Re: Snapping issue
On Sun, Nov 21, 2021 at 02:09:53AM -0700, Jaimos Skriletz wrote: > On Sat, Nov 20, 2021 at 5:22 PM Dominik Vogt wrote: > > 2) Please break up the calculations in DoSnapGrid into multiple > >lines unsing temp variable. It's too hard to read = hard to > >maintain. The previous un-readability of the code shouldn't be > >the standard we aim at. ;-) > > I tried to clean this up a bit, was mostly just cutting/pasting code. > > > 3) Same for "smap_mon = ...". Ouch. > > Agreed, tried to simplify it a bit. Thanks, looks good. > Returning a single score won't work, since vertical and horizontal are > treated separately. Yeah. There are many things I'd like to clean up in the snapattraction and the move code. The code duplication for x + y is one of them. The whole move_resize.c should use the types position, rectangle and size_rect. But that's not important at the moment. (P.S.: And the "xl" and "yt" named drive me crazy.) > > 5) Edge resistance is a kind of reverse snapping with a different > >snap distance: A window that has been moved past the screen > >edge "snaps" back until it has moved more than the edge > >resistance past the edge. This is exactly what snapattraction > >does, just with the snap distance from a different variable. > > > >Can that be merged with the new code? > > This ended up being a bit more complicated, since the resistance isn't > the same as snap attraction (since snap attraction snaps both > left/right but resistance is only one direction). But I was able to > update the monitor function to be able to now deal with both snapping > and resistance by passing it a few bools to control its behavior. I > also took advantage of the new info about if a monitor edge is an > outside or inside edge to distinguish resistance between monitors and > between pages. Though the code is a bit more complicated, I hope it is > readable. Looks much cleaner now! > > I.e. the top/left/right/bottom border of a window shouldn't snap > > to the bottom/right/left/top border of a windows if that latter is > > aligned with a screen edge. > > > > I have added a final check in DoSnapWindow that ignores snapping to > windows on the monitor edge. I think this fixes the issue. Great, that bug is gone. Overall, in a quick test of the situation that made me start this thread, all the annoyances are gone. It behaves totally predictable and glitch free now. The code is much cleaner, easier to read and the indentation is sane. I'm good with committig this patch to master. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Snapping issue
On Sun, Nov 21, 2021 at 02:09:53AM -0700, Jaimos Skriletz wrote: > On Sat, Nov 20, 2021 at 5:22 PM Dominik Vogt wrote: > > > > > > 1) Can we safely assume that the "bool" C type is available in > >every relevant compiler? > > The bool type is used elsewhere in the fvwm code, so I would assume it is > okay. Only in a few places at the moment, and these are all related to new code. The X11 type "Bool" is used in many places though. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Idea: "elastic" move
On Sun, Nov 21, 2021 at 10:11:57PM +, Thomas Adam wrote: > > - Replace the pointer with an invisible one. > > - Warp the pointer to the middle of the screen. There it can > > always be moved in all directions. > > - If the pointer gets too close to the page edges, warp it back > > to the middle of the screen. > > - At the end of the move, warp it back to the original > > position relative to the window. > >(- Maybe a fake cursor can be drawn at the original position so > > that the user does not get confused.) > > So... if I understand this correctly, you would have something like this: > >+--+ >|S | >| | >|..| >|::| >|::| >|::| >|::| >|::| >|::| >|::| >|: P:| >|::| >|::| >| +-:--X---+ :| >| | : | :| >| | : | :| >| | : | :| >| | : W| :| >| | : | :| >| | ::| >| ++ | >| | >+--+ Yes. In addition "X" if the spon on the winode title where the pointer was when it started to move the window, i.e. the position of the fake pointer during the move. > If I'm moving window W interactively, pointer (P) is moved to the middle of > the screen, if I move it right, the window follows that -- but at the distance > of the window's top border and where the pointer is, presumably? Yes. The window (and the "X") make the same moves as the pointer. The offset between pointer and window stays the same. > What if I want to have window W moved to the top-right of the screen (S)? > Wouldn't my pointer end up being at the top of the screen before the window > could reach it, and hence the pointer would be warped back to the middle of > screen S? Yes, when the pointer comes too close to any border (leaves the dotted area on the sketch), it's warped back by the __move_loop to the original "P" in the sketch. Thus it can continue moving in all directions. Of course, the window does not follow the warp. The (P)ointer to (W)indow offset is internally adjusted to a new value. Your other comments make sense. There are no answers at the moment. First we need to play with it and see how it feels. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] parser branches ready for commit
On Sun, Nov 21, 2021 at 09:23:20PM +, Thomas Adam wrote: > On Sun, Nov 21, 2021 at 02:38:09PM +0100, Dominik Vogt wrote: > > The parser branch is a ready as it will be without people testing > > it more. The upstream branch dv/master hat the latest, merged > > together patches with extensive debug to stderr enabled. > > > > Please take a look. If there are no findings I'd like to put this > > on master. > > Before merging, it would be nice to: New version upstream on the dv/master branch (rebased). > * Convert some/all of the printf(stderr, ...) Done. > * Remove/convert some remaining comments starting "!!!". Mostly done, except a couple of comments where there's still work to do. > * Decide on the '#if 1' blocks Done, all gone. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] parser branches ready for commit
On Sun, Nov 21, 2021 at 09:23:20PM +, Thomas Adam wrote: > On Sun, Nov 21, 2021 at 02:38:09PM +0100, Dominik Vogt wrote: > > The parser branch is a ready as it will be without people testing > > it more. The upstream branch dv/master hat the latest, merged > > together patches with extensive debug to stderr enabled. > > > > Please take a look. If there are no findings I'd like to put this > > on master. > > I'll keep testing it here over the next few days. > > Before merging, it would be nice to: > > * Convert some/all of the printf(stderr, ...) calls to fvwm_debug() lines so > debugging is going through that mechanism. ... Yes, no problem. The debug printfs were meant to go away before a real commit, but that won't be happening soon. > * Remove/convert some remaining comments starting "!!!". The '!!!' is my way of marking the places that are incomplete or need work. It will take a while to get these done. I'd > * Decide on the '#if 1' blocks -- are there they to stay permanently? No. Any block marked with !!! must go away. I need to understand the single one that is not, but I think it must stay without the "if". > * Not just related to change per se (but still present) appears to be what I > would class as extraneous "return;" lines at the end of void functions which > seems odd to me. I always put a return at the end of a function. So, if there is a void function without a return, function is likely incomplete or buggy. > I will also get some tests in place as well. I thought we could have some litte suite: 1) A shell script that starts Xephyr on desk 99 or whatever. 2) Starts fvwm on it with "-f testconfig". 3) testconfig is a simple script that adds a bit of test framework like printing the a test name and providing some result testing command. 4) Tests that do they work in a standardised way using the framework. See the three attached parsing test cases for nesting, forking and self modifying functions. > > (Note: Produces a ton of debug output and is not suitable to the > > general audience. Debug output can be disabled by debug defines > > in functions.c and cmdparser_old.c.) > > It's things like this which tell me "it's not quite ready yet to be merged". > Which is good, because this branch is level with master so it doesn't need to > be merged just to test it -- and it's that shift in mentality, trying to > keep master in the most stable state it can be which helps a lot with the > release process. Yeah, agreed. Still sometimes it's necessary to get some help from the people who would test master but not obscure development branches. This mess of interleaving patches dealing with all kinds of topics is not what I'd accept nowadays in a stable branch (i.e. now that there's git). At the moment it can hardly be avoided because there are so many fundamental changes going on. Ciao Dominik ^_^ ^_^ -- Dominik Vogt # test nesting calls destroyfunc ftest addtofunc ftest + i ftest ftest ftest # test fork bomb destroyfunc ftest addtofunc ftest + i ftest + i ftest ftest ftest # test function growth destroyfunc ftest addtofunc ftest + i ftest2 destroyfunc ftest2 addtofunc ftest2 + i addtofunc ftest i ftest2 ftest ftest
Idea: "elastic" move
Duing the the discussion about annoying page flipping during interactive move Thomas said that he switched it off because it was annoying and that the pages then felt like disconnected desktops. This is precisely true and matches my own opinion. Fvwm should be better than that. The big, paged virtual desktop should not feel like disjoint desktops but like a big sketch block with plenty of space. => Make crossing the page boundaries easier and intuitive without being annoying. Situation: At the moment, interactive move with the mouse feels awkward. Pushing windows past the page edge often fails because the pointer hits the edge and cannot move further. It's annoying and uncomfortable. The "pointer hits page border" this is a real show stopper. -- This is the idea (working title "elastic paging"): 1. When an interactive move starts: - Replace the pointer with an invisible one. - Warp the pointer to the middle of the screen. There it can always be moved in all directions. - If the pointer gets too close to the page edges, warp it back to the middle of the screen. - At the end of the move, warp it back to the original position relative to the window. (- Maybe a fake cursor can be drawn at the original position so that the user does not get confused.) Note: Its then possible to move a window any distance without releasing the pointer. Note 2: As a side effect, windows can also be pushed over the desktop borders (maybe enforce that at least a couple of pixels remain visible). 2. Remove timed paging during interactive move completely (EdgeMoveDelay style) 3. Instead, when you push the original pointer position off page the viewport automatically starts scrolling to keep that position (the fake pointer) visible. Think of the current viewport position being connected with the original one with a rubber band: If you change you mind and move the window back towards its original position, the viewport also moves back, relaxing the rubber band. (When windows on other pages become visible, snapattraction automatically allows to snap to them.) 4. When the move ends, the window is placed and the viewport snaps back to a full page. It may be a bit tricky to choose the proper page automatically. It's easy if the window has completely moved to another page, but what if it's stuck between two or even four pages? 5. Visual feedback (optional) - If the viewport jumps use some kind of smooth animation. This may be especially important if the viewport snaps to a full page when the move ends. - See how the fake pointer can be implemented. - Add a new "activity" colour to window decorations. This colour may be used while a long running action affects a window (interactive move/resize etc.). - The "activity" colour could also be used when a window border hits the page edge during an interactive move. Say, if the window is already at the edge and you keep "pushing", that border uses the "activity" colour to indicate that something is happening. 6. Future - Also do something about interactive resize. - If elastic paging works nicely, it should become the default behaviour. To disable it, you have to turn it off explicitly - an empty config still has it on. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[RFC] parser branches ready for commit
The parser branch is a ready as it will be without people testing it more. The upstream branch dv/master hat the latest, merged together patches with extensive debug to stderr enabled. Please take a look. If there are no findings I'd like to put this on master. The patches also contain: * Removal of the Repeat command. * The discussed limitations on function size and recursion, solving the "fork bomb" problem and inifinitely growing, self modifying functions. (Note: Produces a ton of debug output and is not suitable to the general audience. Debug output can be disabled by debug defines in functions.c and cmdparser_old.c.) Ciao Dominik ^_^ ^_^ -- Dominik Vogt
dv Branches
Okay, git access works again. I'll do further development on dv/devel or on private topic branches, then put them in dv/master before considering to push them. With so much disruptive work pending, we may want to introduce a devel branch upstream to reduce pollution of the master? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (6) Man page changes -- second attempt
On Sat, Nov 20, 2021 at 02:37:29PM -0700, Jaimos Skriletz wrote: > Lintian found a few minor typos in the manual pages. Patch attached. Thanks; applied them to my working branch. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
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: Snapping issue
And there's still a bug that had been present in the old code: Assume you have a window on screen and a button bar aligned with the bottom of the screen. Grab the window at the top border and move it past the button bar towards the bottom of the screen. When the top of the window gets close to the bottom screen edge, it snaps to the bottom of the button bar: +-- | | | | | | | page 0 0 | | | | | | | |-| | button bar | +---+--+--+ | | | window | | | +--+ page 0 1 I.e. the top/left/right/bottom border of a window shouldn't snap to the bottom/right/left/top border of a windows if that latter is aligned with a screen edge. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Snapping issue
On Sat, Nov 20, 2021 at 04:17:41PM -0700, Jaimos Skriletz wrote: > On Sun, Nov 14, 2021 at 10:57 AM Dominik Vogt wrote: > > > > The snapping code is really an unmaintainable mess. Some thoughts > > on how it might be rewritten: > > > > Here are some thoughts I have, you can see them in js/snapattract branch. > > I have split the three different snapping checks into individual > functions for Monitor Edges, Windows, and Grid. I have then set up a > priority of Monitor > Windows > Grid, so this way when something is > found to snap too, it stops looking for other things based on the > priority, and this should stop some of the strange interactions > between deciding what to snap too. I also treat horizontal and > vertical independently so it can snap to both a window and a monitor > edge in different directions. Currently the priority is hardcoded, but > since the checks are now individual functions, it could be possible to > set it up so the user can configure what snap priority. Comments > welcome. Thanks! It looks much cleaner now (not tested though). Some comments: 1) Can we safely assume that the "bool" C type is available in every relevant compiler? 2) Please break up the calculations in DoSnapGrid into multiple lines unsing temp variable. It's too hard to read = hard to maintain. The previous un-readability of the code shouldn't be the standard we aim at. ;-) 3) Same for "smap_mon = ...". Ouch. 4) I'm not sure whether screen boundaries should have a higher priority than windows and not equal priorities. That would require scoring. For example, if a window on (0 0) is fullscreen so that the borders are on the adjacent pages. Then you couldn't align a window on (0 1) with that window border because snapping to the screen edge always wins. Shouldn't be too difficult to implement; the new sub functions would simply return a score = the absolute snapping distance that was used, and the lowest one wins. (Gird is still only used if there was no other snapping.) 5) Edge resistance is a kind of reverse snapping with a different snap distance: A window that has been moved past the screen edge "snaps" back until it has moved more than the edge resistance past the edge. This is exactly what snapattraction does, just with the snap distance from a different variable. Can that be merged with the new code? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
git access
Sending patches is getting out of hand. I believe I had once write access to the git repo, but it doesn't work anymore (permission denied when updating). What do I need to do to reactivate access? (A quick recap of the procedure involved in getting patches on master wold help too. I'd also be happy with my own branches without committing to master myself.) Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[PATCH/RFC] Xinerama
On Sat, Nov 20, 2021 at 10:24:46PM +, Thomas Adam wrote:
> On Sat, Nov 20, 2021 at 03:54:10PM +0100, Dominik Vogt wrote:
> > On Sat, Nov 20, 2021 at 02:15:58PM +, Thomas Adam wrote:
> > > On Sat, Nov 20, 2021, 14:13 Dominik Vogt wrote:
> > >
> > > > Is Xinerama still useful for anything or can we remove it?
> > > >
> > > It has already been removed. Where are you seeing references to it?
Patch attached. Can you please double check and fill in the gaps?
(see comments belo).
> > ./fvwm/menus.c: fXineramaRoot = False;
> > ./fvwm/menus.c: else if (StrEquals(tok,"xineramaroot"))
Not sure what the right name of the option and the flag variable
should be. There's only a placeholder in the man page either.
> > ./perllib/FVWM/Commands.pm: descr => q{Move a window to another
> > Xinerama screen},
[MOVE_TO_SCREEN]
This one should be just renamed to just "screen"?
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 03149ce8658894f22e3c5e17cc8d981bb9563ec8 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sun, 21 Nov 2021 00:27:00 +0100
Subject: [PATCH 1/4] Remove Xinerama leftovers.
---
bin/fvwm-config.in | 6 +++---
doc/fvwm3_manpage_source.adoc| 5 -
fvwm/add_window.c| 3 +--
fvwm/commands.h | 10 -
fvwm/functable.c | 2 +-
fvwm/fvwm.h | 8 +++
fvwm/menuparameters.h| 6 ++
fvwm/menus.c | 27 +++
fvwm/module_interface.h | 1 -
fvwm/move_resize.c | 15 ++---
fvwm/placement.c | 4 ++--
fvwm/screen.h| 1 -
fvwm/style.c | 24 ++---
fvwm/style.h | 8 +++
fvwm/update.c| 6 --
fvwm/windowlist.c| 2 +-
libs/FTips.c | 2 +-
libs/defaults.h | 6 --
modules/FvwmButtons/FvwmButtons.h| 1 -
modules/FvwmForm/FvwmForm.c | 17 ---
perllib/FVWM/Commands.pm | 32 +---
perllib/FVWM/Tracker/GlobalConfig.pm | 7 +++---
22 files changed, 59 insertions(+), 134 deletions(-)
diff --git a/bin/fvwm-config.in b/bin/fvwm-config.in
index 77ea48a3..4ca9edce 100644
--- a/bin/fvwm-config.in
+++ b/bin/fvwm-config.in
@@ -90,9 +90,9 @@ with_shape=@with_shape@
with_shm=@with_shm@
with_sm=@with_sm@
with_xcursor=@with_xcursor@
-with_xinerama=@with_xinerama@
with_xft=@with_xft@
with_xpm=@with_xpm@
+with_xrandr=@with_xinerama@
with_xrender=@with_xrender@
while test $# -gt 0; do
@@ -162,9 +162,9 @@ while test $# -gt 0; do
test "$with_shm"= "yes" && echo "shm"
test "$with_sm" = "yes" && echo "sm"
test "$with_xcursor"= "yes" && echo "xcursor"
- test "$with_xinerama" = "yes" && echo "xinerama"
test "$with_xft"= "yes" && echo "xft"
test "$with_xpm"= "yes" && echo "xpm"
+ test "$with_xrandr" = "yes" && echo "xrandr"
test "$with_xrender"= "yes" && echo "xrender"
;;
@@ -210,9 +210,9 @@ while test $# -gt 0; do
echo " sm (session management): $with_sm"
echo " rsvg (SVG icons and images): $with_rsvg"
echo " xcursor (ARGB/animated cursors): $with_xcursor"
- echo " xinerama (multi-head): $with_xinerama"
echo " xft (FreeType anti-alias font): $with_xft"
echo " xpm: $with_xpm"
+ echo " xrandr (multi-head): $with_xrand"
echo " xrender (XFree86 Xrender extention): $with_xrender"
;;
diff --git a/doc/fvwm3_manpage_source.adoc b/doc/fvwm3_manpage_source.adoc
index 42187d14..dee9a0e0 100644
--- a/doc/fvwm3_manpage_source.adoc
+++ b/doc/fvwm3_manpage_source.adoc
@@ -2264,6 +2264,9 @@ The _context-rectangle_ can be one of:
_Root_:::
the root window of the current screen.
+
+_ScreenRoot_:::
+ !!!???
++
_Mouse_:::
a 1x1 rectangle at the mouse position.
+
@@ -6130,7 +6133,7 @@ it is still possible to move or resize windows across the edge of the
current screen. See also *EdgeThickness*.
The option _EdgeMoveResistance_ makes it easier to place a window
-directly adjacent to the screen's or xinerama screen's border. It
+directly adjacent to the screen's border. It
takes one or two parameters. The first parameter tells how many pixels
over the edge of the screen a window's edge must move before it
actually moves partia
fork-bomb like functions
I wonder if we should do something about these kind of functions: Theres the definition "MAX_FUNCTION_DEPTH 512" in defaults.h that prevents functions from nesting infinitely deep: addtofunc foo i foo foo => ok But this is not good: addtofunc foo + i foo + i foo h hangs: foo Although functions above the maximum depth abort, it still executes about 2^512 leaf functions before finally exiting, i.e. it never completes. I could add an execution counter that limits the total number of command lines that can be executed from a single top level function call; maybe limit that to 1000. -- Another problem: It's possible to add items to functions that are currently in use. addtofunc foo i bar addtofunc bar i addtofunc foo i bar # hangs: foo Not sure if we should just forbid to add items to running functions. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[PATCH] (9) master and new parser patches
A couple of small changes for master:
0001: Makes the code in log.c easiert to read.
0002: Ignore emacs backup files.
0003: Clean up a header file.
0004: Remove the unused dependency from libs/vpacket.h to fvwm/fvwm.h.
0005: Your NEW-PARSEER.md patch without the reindentation.
0006: Remove the -D command line option. I'm not sure if we should
really delete this. Should probably rather be a "bugopts" option.
parser branch on top of master:
0007-0009: Current cleaned up version of the parser parches
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 3f61e78201a452a69c74ccde036e371391e0dfd3 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 14:39:54 +0100
Subject: [PATCH 1/9] Clean up log.c.
---
libs/log.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/libs/log.c b/libs/log.c
index 23f28dcb..877a9f74 100644
--- a/libs/log.c
+++ b/libs/log.c
@@ -55,35 +55,36 @@ void
log_open(const char *fvwm_userdir)
{
char *path, *file_name;
+ char *expanded_path;
if (lib_log_level == 0)
return;
- if (log_file_name != NULL &&
- log_file_name[0] == '-' && log_file_name[1] == 0) {
+ /* determine file name or file path to use */
+ file_name = log_file_name;
+ if (file_name == NULL)
+ {
+ file_name = getenv("FVWM3_LOGFILE");
+ }
+ if (file_name == NULL)
+ {
+ file_name = FVWM3_LOGFILE_DEFAULT;
+ }
+ /* handle stderr logging */
+ if (log_file_name[0] == '-' && log_file_name[1] == 0)
+ {
log_file = stderr;
return;
}
- if ((file_name = log_file_name) == NULL)
- file_name = getenv("FVWM3_LOGFILE");
-
- if (file_name != NULL)
+ /* handle file logging */
+ expanded_path = expand_path(file_name);
+ if (expanded_path[0] == '/')
{
- char *expanded_path;
-
- expanded_path = expand_path(file_name);
- if (expanded_path[0] == '/')
- {
- path = expanded_path;
- }
- else
- {
- xasprintf(, "%s/%s", fvwm_userdir, expanded_path);
- free((char *)expanded_path);
- }
+ path = expanded_path;
}
else
{
- xasprintf(, "%s/%s", fvwm_userdir, FVWM3_LOGFILE_DEFAULT);
+ xasprintf(, "%s/%s", fvwm_userdir, expanded_path);
+ free((char *)expanded_path);
}
log_close();
--
2.30.2
From 7ff0553f89bf5b767a5f67d8bcbdc1c63363a01d Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 11:04:03 +0100
Subject: [PATCH 2/9] .gitignore: Ignore emacs backups.
---
.gitignore | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.gitignore b/.gitignore
index 7ec13d8d..6c538bee 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,8 @@
*.o
*.patch
*~
+*#
+.#*
Makefile
Makefile.in
aclocal.m4
--
2.30.2
From 2021ba60dc174b9508f87d4ee7ffb800792cd0ab Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 12:12:13 +0100
Subject: [PATCH 3/9] Header cleanup.
---
libs/vpacket.h | 10 --
1 file changed, 10 deletions(-)
diff --git a/libs/vpacket.h b/libs/vpacket.h
index d0d88b2d..bd549fab 100644
--- a/libs/vpacket.h
+++ b/libs/vpacket.h
@@ -16,7 +16,6 @@
This is the same packet as the M_ADD_WINDOW packet, the
only difference being the type.
*/
-/* RBW- typedef struct config_win_packet */
typedef struct ConfigWinPacket
{
/*** Alignment notes ***/
@@ -33,14 +32,7 @@ typedef struct ConfigWinPacket
unsigned long desk;
unsigned long monitor_id;
unsigned long monitor_name;
- /*
- Temp word for alignment - old flags used to be here.
- - remove before next release.
- RBW - 05/01/2000 - layer has usurped this slot.
- unsigned long dummy;
- */
unsigned long layer;
-
unsigned long hints_base_width;
unsigned long hints_base_height;
unsigned long hints_width_inc;
@@ -56,8 +48,6 @@ typedef struct ConfigWinPacket
unsigned long hints_win_gravity;
unsigned long TextPixel;
unsigned long BackPixel;
-
- /* Everything below this is post-GSFR */
unsigned long ewmh_hint_layer;
unsigned long ewmh_hint_desktop;
unsigned long ewmh_window_type;
--
2.30.2
From 500e6f4b2c532d76581ba055d216a5d63cc93a03 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 11:50:52 +0100
Subject: [PATCH 4/9] Remove libs/vpacket.h depending on fvwm.h.
---
libs/vpacket.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libs/vpacket.h b/libs/vpacket.h
index bd549fab..98eb590c 100644
--- a/libs/vpacket.h
+++ b/libs/vpacket.h
@@ -3,7 +3,6 @@
#ifndef FVWMLIB_VPACKET_H
#define FVWMLIB_VPACKET_H
#include "fvwm_x11.h"
-#include "fvwm/fvwm.h"
/*
All new-style module packets (i.e., those that are not simply arrays
@@ -70,7 +69,7 @@ typedef struct MiniIconPacket
{
Window w;
Window frame;
- FvwmWindow*fvwmwin;
+ void *fvwmwin;
unsigned long width;
unsigned long height;
unsigned long depth;
--
2.30.2
From 01686e05071f7fbd460c73e4e70462136b50eb29 Mon Sep 17 00:00:00 2001
From: Thomas Adam
Date: Thu, 18 Nov 2
Re: Xinerama
On Sat, Nov 20, 2021 at 03:54:10PM +0100, Dominik Vogt wrote: > On Sat, Nov 20, 2021 at 02:15:58PM +, Thomas Adam wrote: > > On Sat, Nov 20, 2021, 14:13 Dominik Vogt wrote: > > > > > Is Xinerama still useful for anything or can we remove it? > > > > > It has already been removed. Where are you seeing references to it? > > Everywhere. I'll remove the junk on the parser branch. No sorry, too complex for me, and I don't know the new implementation. The code in menus.c, move_resize.c etc is non-trivial and maybe still necessary. But wen can't do it with the parser branch pending. It would cause too many conflicts. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: Xinerama
On Sat, Nov 20, 2021 at 02:15:58PM +, Thomas Adam wrote: > On Sat, Nov 20, 2021, 14:13 Dominik Vogt wrote: > > > Is Xinerama still useful for anything or can we remove it? > > > It has already been removed. Where are you seeing references to it? Everywhere. I'll remove the junk on the parser branch. $ rgrep -i xinerama . 2> /dev/null ./libs/defaults.h:/*** Xinerama ***/ ./libs/defaults.h:#define DEFAULT_XINERAMA_ENABLEDTrue /* Xinerama on by default */ ./libs/defaults.h:#define XINERAMA_CONFIG_STRING "XineramaConfig" ./libs/FTips.c: fscreen_scr_arg *fsarg = NULL; /* for now no xinerama support */ ./modules/FvwmForm/FvwmForm.c: if (strncasecmp(buf, XINERAMA_CONFIG_STRING, ./modules/FvwmForm/FvwmForm.c:sizeof(XINERAMA_CONFIG_STRING)-1) == 0) ./modules/FvwmForm/FvwmForm.c:FScreenConfigureModule(buf + sizeof(XINERAMA_CONFIG_STRING)-1); ./modules/FvwmForm/FvwmForm.c: buf, XINERAMA_CONFIG_STRING, sizeof(XINERAMA_CONFIG_STRING)-1) ./modules/FvwmForm/FvwmForm.c: FScreenConfigureModule(buf + sizeof(XINERAMA_CONFIG_STRING)-1); ./modules/FvwmButtons/FvwmButtons.h:void handle_xinerama_string(char *args); ./.git/rr-cache/1c03ad931074cd97196af43ae2fc0134a13171cd/preimage:directly adjacent to the screen's or xinerama screen's border. It ./.git/rr-cache/d3fcccba50db8b879e64033b3c5f2ebe88fa6f57/preimage:directly adjacent to the screen's or xinerama screen's border. It ./doc/fvwm3_commands.ad:directly adjacent to the screen's or xinerama screen's border. It ./doc/fvwm3all.1:directly adjacent to the screen\(cqs or xinerama screen\(cqs border. It ./doc/fvwm3_styles.ad:directly adjacent to the screen's or xinerama screen's border. It ./doc/fvwm3styles.1:directly adjacent to the screen\(cqs or xinerama screen\(cqs border. It ./doc/fvwm3_manpage_source.adoc:directly adjacent to the screen's or xinerama screen's border. It ./fvwm/windowlist.c: * because it sets the xinerama screen origin */ ./fvwm/add_window.c:fw->edge_resistance_xinerama_move = ./fvwm/add_window.c:pstyle->edge_resistance_xinerama_move; ./fvwm/move_resize.c:* in case Xinerama is used. */ ./fvwm/move_resize.c: /* Resist moving windows between xineramascreens */ ./fvwm/move_resize.c: if (fw->edge_resistance_xinerama_move) ./fvwm/move_resize.c: scr_x1 + fw->edge_resistance_xinerama_move) ./fvwm/move_resize.c: fw->edge_resistance_xinerama_move) ./fvwm/move_resize.c: scr_y1 + fw->edge_resistance_xinerama_move) ./fvwm/move_resize.c: fw->edge_resistance_xinerama_move) ./fvwm/style.c: if (add_style->flags.has_edge_resistance_xinerama_move) ./fvwm/style.c: SSET_EDGE_RESISTANCE_XINERAMA_MOVE( ./fvwm/style.c: SGET_EDGE_RESISTANCE_XINERAMA_MOVE(*add_style)); ./fvwm/style.c: unsigned has_xinerama_move; ./fvwm/style.c: has_xinerama_move = 0; ./fvwm/style.c: has_xinerama_move = 1; ./fvwm/style.c: has_xinerama_move = 0; ./fvwm/style.c: ps->flags.has_edge_resistance_xinerama_move = ./fvwm/style.c: has_xinerama_move; ./fvwm/style.c: ps->flag_mask.has_edge_resistance_xinerama_move = 1; ./fvwm/style.c: ps->change_mask.has_edge_resistance_xinerama_move = 1; ./fvwm/style.c: SSET_EDGE_RESISTANCE_XINERAMA_MOVE(*ps, val[1]); ./fvwm/fvwm.h: char*IconScreen; /* Xinerama screen */ ./fvwm/fvwm.h: unsigned has_edge_resistance_xinerama_move : 1; ./fvwm/fvwm.h: int edge_resistance_xinerama_move; ./fvwm/fvwm.h: int edge_resistance_xinerama_move; ./fvwm/commands.h: F_XINERAMA, ./fvwm/commands.h: F_XINERAMAPRIMARYSCREEN, ./fvwm/commands.h: F_XINERAMASLS, ./fvwm/commands.h: F_XINERAMASLSSCREENS, ./fvwm/commands.h: F_XINERAMASLSSIZE, ./fvwm/commands.h:void CMD_Xinerama(F_CMD_ARGS); ./fvwm/commands.h:void CMD_XineramaPrimaryScreen(F_CMD_ARGS); ./fvwm/commands.h:void CMD_XineramaSls(F_CMD_ARGS); ./fvwm/commands.h:void CMD_XineramaSlsScreens(F_CMD_ARGS); ./fvwm/commands.h:void CMD_XineramaSlsSize(F_CMD_ARGS); ./fvwm/screen.h:unsigned has_xinerama_state_changed : 1; ./fvwm/module_interface.h:void broadcast_xinerama_state(void); ./fvwm/placement.c: * Xinerama screen. 1: The intuitive way giving a geometry ./fvwm/placement.c: * 2: Do NOT specify a Xinerama screen (or specify it to be ./fvwm/functable.c: /* - Move a window to another Xinerama screen */ ./fvwm/style.h:#define SGET_EDGE_RESISTANCE_XINERAMA_MOVE(s) \ ./fvwm/style.h: ((s).edge_resistance_xinerama_move) ./fvwm/style.h:#define SSET_EDGE_RESISTANCE_XINERAMA_MOVE(s,x) \ ./fvwm/style.h: ((s).edge_resistance_xinerama_move = (x)) ./fvwm/menuparameters.h:/* A position on the Xinerama screen on whic
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
Xinerama
Is Xinerama still useful for anything or can we remove it? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
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 = r
[PATCH] (6) Various small changes
For master:
0001: Fix uninitialised variables in lib.
0002: Remove "-blackout" option.
0003: Docuement -v and alias it to --verbose.
0004: Don't list all options in the SYNOPSIS.
0005: Change getpwuid.c interface (for next patch)
0006: Implement -o/--output-logfile option.
If given, use that logfile (abs or relative path)
If the path is just "-", use stderr
If not present use file from env variable
If not presen, use default.
fvwm -v -o - ... # to stderr
fvwm -v -o //home/foo/bar # to file
The patch also fixes a memory leak.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From c81eb2cf991a5af850850ecf87d38bf6851bd788 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 00:31:31 +0100
Subject: [PATCH 1/6] Fix uninitialised varibles.
---
libs/FScreen.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libs/FScreen.c b/libs/FScreen.c
index 32ee78a7..ffe4d4bb 100644
--- a/libs/FScreen.c
+++ b/libs/FScreen.c
@@ -806,10 +806,10 @@ void FScreenTranslateCoordinates(
int FScreenClipToScreen(fscreen_scr_arg *arg, fscreen_scr_t screen,
int *x, int *y, int w, int h)
{
- int sx;
- int sy;
- int sw;
- int sh;
+ int sx = 0;
+ int sy = 0;
+ int sw = 0;
+ int sh = 0;
int lx = (x) ? *x : 0;
int ly = (y) ? *y : 0;
int x_grav = GRAV_POS;
@@ -852,10 +852,10 @@ int FScreenClipToScreen(fscreen_scr_arg *arg, fscreen_scr_t screen,
void FScreenCenterOnScreen(fscreen_scr_arg *arg, fscreen_scr_t screen,
int *x, int *y, int w, int h)
{
- int sx;
- int sy;
- int sw;
- int sh;
+ int sx = 0;
+ int sy = 0;
+ int sw = 0;
+ int sh = 0;
int lx;
int ly;
@@ -895,10 +895,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
From 31b4637b705a1ff567b9846a77dd284e29b069ef Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 00:32:44 +0100
Subject: [PATCH 2/6] Remove obsolete option "-blackout".
---
doc/fvwm3_manpage_source.adoc | 7 ---
fvwm/fvwm3.c | 7 ---
2 files changed, 14 deletions(-)
diff --git a/doc/fvwm3_manpage_source.adoc b/doc/fvwm3_manpage_source.adoc
index e9642463..8509b27f 100644
--- a/doc/fvwm3_manpage_source.adoc
+++ b/doc/fvwm3_manpage_source.adoc
@@ -4,7 +4,6 @@
_config-file_] [*-r*] [*-s* [_screen_num_]] [*-V*] [*-C* _visual-class_
| *-I* _visual-id_] [*-l* _colors_ [*-L*] [*-A*] [*-S*] [*-P*]] [*-D*]
[*-h*] [*-i* _client-id_] [*-F* _state-file_] [*--debug-stack-ring*]
-[*-blackout*]
== DESCRIPTION
@@ -234,12 +233,6 @@ default, when fvwm does not need a color any more it frees this color so
that a new color can be used. This option may speed up image loading and
save a few bits of memory.
-*-blackout*::
-
-This option is provided for backward compatibility only. Blacking out
-the screen during startup is not necessary (and doesn't work) anymore.
-This option will be removed in the future.
-
*--debug-stack-ring*::
Enables stack ring debugging. This option is only intended for internal
diff --git a/fvwm/fvwm3.c b/fvwm/fvwm3.c
index 90acfe13..7f4db058 100644
--- a/fvwm/fvwm3.c
+++ b/fvwm/fvwm3.c
@@ -1931,13 +1931,6 @@ int main(int argc, char **argv)
usage(1);
exit(0);
}
- else if (strcmp(argv[i], "-blackout") == 0)
- {
- /* obsolete option */
- fvwm_debug(__func__,
- "The -blackout option is obsolete, it will be "
- "removed in 3.0.");
- }
else if (strcmp(argv[i], "-r") == 0 ||
strcmp(argv[i], "-replace") == 0 ||
strcmp(argv[i], "--replace") == 0)
--
2.30.2
From 6d4a0afc69f7ebe7c829025e220b40b43487137f Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Sat, 20 Nov 2021 00:44:58 +0100
Subject: [PATCH 3/6] Document -v option and alias it to --verbose.
---
doc/fvwm3_manpage_source.adoc | 2 +-
fvwm/fvwm3.c | 5 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/doc/fvwm3_manpage_source.adoc b/doc/fvwm3_manpage_source.adoc
index 8509b27f..f647dffa 100644
--- a/doc/fvwm3_manpage_source.adoc
+++ b/doc/fvwm3_manpage_source.adoc
@@ -238,7 +238,7 @@ save a few bits of memory.
Enables stack ring debugging. This option is only intended for internal
debugging and should only be used by developers.
-*-v*::
+*-v* | *--verbose*::
Enables debug logging. Writes in append mode to fvwm log file, which is
~/.fvwm/fvwm3-output.log by default. See ENVIRONMENT section on how to
diff --git a/fvwm/fvwm3.c b/fvwm/fvwm3.c
index 7f4db058..972c208d 100644
--- a/fvwm/fvwm3.c
+++ b/fvwm/fvwm3.c
@@ -1246,6 +1246,7 @@ static void usage(int is_verbose)
" -r: replace running window manager\n"
" -s [screen]: manage a single
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: [RFC] Fake a global monitor when RandR is not available.
On Fri, Nov 19, 2021 at 01:48:51PM +, Thomas Adam wrote: > On Fri, Nov 19, 2021 at 02:53:32AM +0100, Dominik Vogt wrote: > > 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. > > I think most people have started to use Xephyr instead of Xnest, which does > support RANDR Good. Works better than xnest. Forget the patch and thanks for the hint. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[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_
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
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 Wed, Nov 17, 2021 at 10:40:56PM +, Thomas Adam wrote: > On Wed, Nov 17, 2021 at 08:40:09PM +0100, Dominik Vogt wrote: > > 'k, the patched code didn't immediately crash, so here it is (two > > patches). Please test. > > I've applied those two patches on a branch called `new-parser`. > > So far, I've tested this on approximately five different configuration files > and haven't noticed anything unusual or anything which breaks. Which is a > good sign. 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. 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. > > The new code: > >... > This is sensible, and from a quick glance at what's there at the moment, it > makes sense. I also need to remember what this is all good for. Fortunately the hooks and structures are documented (cmdparser_hooks.h, cmdparser.h), and the whole code is only several hundred lines of not so bad code. The vital parts in functions.c are DeferExecution(), __execute_command_line() and execute_complex_function() which are all just 300 lines of code each. > I'm hoping that we can also derive the execute_context_t from > the parsed command as well, 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. > I did wonder whether we might want to consider yacc/bison for the grammar of > the commands, but I've never personally been a fan of it, but it could help > wih some of the raw parsing... 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. 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. * Some way to store it in the function table (at compile time without making functable.h depend on all other .h files.) * 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?). * An enhanced parser that triggers BNF parsing. Alternative: functions call the parser themselves and the syntax description can be kept in the local file. * The parsing code itself. * Memory management for parsed strings etc. * Rewritten functions (starting with simple ones first). * Later: An idea how to tackle meta command mosters like Style, MenuStyle or MoveResizeMaximize. > > * The "repeat" command may cause crashes or leaks. > > Weren't we thinking of removing the repeat command at one point? Yes. There was a patch on the onld branch to do that. I reverted it twelve minues after making that commit for an unknown reason. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[RFC] New parser framework for testing
'k, the patched code didn't immediately crash, so here it is (two patches). Please test. What this is about: > The whole point in the completed part of the work was to make the > command line parser pluggable. As far as I remember, that part > was done, but needed more testing by users. Now that the rebase > potentially contains some mistakes and may miss some things that > have changed in between, it's even more important. > > There is a slight incompatibility of the new code with some thiung > about quoting plus variable expansion. I had started a thread on > the mailing list and asked if that would annoy anybody and the > people who replied said no. Anyway (a) I cannot remember what it > was exactly (we can dig it up from the mail archive) and (b) there > won't be any progress if we make a new parser compatible with the > crappy bugs of the old one. The new code: * Rewrites functions.c to use hooks that are provided by a pluggable parser module (even at run time). * The module that replicates the old parser behaviour is in cmdparser_old.[ch]. * The long term goal is to replace the _old parser with a new one that evauates the BNF definitions and does the parsing of function arguments before actually calling them. * To allow that, a "parsing context" structure is passed to the CMD_... functions. This is already implemented but not used. * How the "parsing context" structure should look like is yet to be defined. * It shoul be entirely possible to convert command functions one by one to the new type of parser. So that word does not need to be done in a single big step. Possible pitfalls: * Watch out for changed whitespace behaviour. * A complex function name cannot start with a builtin name followed by an embedded space. * Possible to-dos in the code (look for the string '!!!'). * Bad behaviour of the fvwm_debug function because of wrong parameters being passed. * The "repeat" command may cause crashes or leaks. * The general code in functions.c may as well. * Same in CMD_... functions and various parts of fvwm that call execute_function to do their work. * Debug code needt to be removed later. Ciao Dominik ^_^ ^_^ -- Dominik Vogt From eddb78f108c0284fd7c670dd4d08811de59d2820 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Wed, 17 Nov 2021 20:03:57 +0100 Subject: [PATCH 1/2] Rewrite parser framework. --- fvwm/Makefile.am | 4 +- fvwm/add_window.c| 9 +- fvwm/builtins.c | 4 +- fvwm/cmdparser.h | 65 fvwm/cmdparser_hooks.h | 74 fvwm/cmdparser_old.c | 424 + fvwm/cmdparser_old.h | 11 + fvwm/colorset.c | 2 +- fvwm/conditional.c | 21 +- fvwm/conditional.h | 42 +++ fvwm/events.c| 40 +- fvwm/ewmh.c | 7 +- fvwm/ewmh_events.c | 33 +- fvwm/expand.c| 48 ++- fvwm/expand.h| 2 +- fvwm/functable.c | 66 +++- fvwm/functable.h | 25 +- fvwm/functable_complex.c | 157 fvwm/functable_complex.h | 55 +++ fvwm/functions.c | 771 ++- fvwm/functions.h | 41 +-- fvwm/fvwm.h | 49 +-- fvwm/fvwm3.c | 12 +- fvwm/menucmd.c | 3 +- fvwm/menus.c | 4 +- fvwm/misc.c | 1 + fvwm/module_interface.c | 3 +- fvwm/move_resize.c | 29 +- fvwm/placement.c | 5 +- fvwm/read.c | 14 +- fvwm/repeat.c| 6 +- fvwm/schedule.c | 3 +- fvwm/update.c| 7 +- fvwm/virtual.c | 24 +- fvwm/windowlist.c| 3 +- 35 files changed, 1373 insertions(+), 691 deletions(-) create mode 100644 fvwm/cmdparser.h create mode 100644 fvwm/cmdparser_hooks.h create mode 100644 fvwm/cmdparser_old.c create mode 100644 fvwm/cmdparser_old.h create mode 100644 fvwm/functable_complex.c create mode 100644 fvwm/functable_complex.h diff --git a/fvwm/Makefile.am b/fvwm/Makefile.am index a4bb6a0b..9ae456f9 100644 --- a/fvwm/Makefile.am +++ b/fvwm/Makefile.am @@ -19,6 +19,7 @@ fvwm3_SOURCES = \ placement.h read.h repeat.h execcontext.h schedule.h screen.h \ session.h stack.h style.h update.h virtual.h window_flags.h frame.h \ infostore.h \ + cmdparser.h cmdparser_hooks.h cmdparser_old.h functable_complex.h \ \ menus.c style.c borders.c events.c move_resize.c builtins.c \ add_window.c icons.c fvwm3.c frame.c placement.c virtual.c \ @@ -28,7 +29,8 @@ fvwm3_SOURCES = \ menubindings.c decorations.c ewmh_icons.c update.c bindings.c misc.c \ cursor.c colormaps.c modconf.c ewmh_conf.c read.c schedule.c \ menucmd.c ewmh_names.c icccm2.c windowshade.c focus_policy.c repeat.c \ - execcontext.c menugeometry.c menudim.c condrc.c infostore.c + execcontext.c menugeometry.c menudim.c condrc.c infostore.c \ + cmdparser_
Re: [PATCH] (6) Man page changes -- second attempt
On Wed, Nov 17, 2021 at 04:40:55PM +, Thomas Adam wrote: > On Wed, Nov 17, 2021 at 02:35:32PM +0100, Dominik Vogt wrote: > > On Tue, Nov 16, 2021 at 01:36:53AM +0100, Dominik Vogt wrote: > > > This is the full set of patches for splitting the man page, to be > > > applied to master. > > > > Second attempt. The style docs are not moved aound in the man > > page. The .section files have been renamed to .ad because > > asciidoc only evaluates "ifdef" in included files if they have a > > known extesion. ".adoc" cannot be used because the pattern rules > > in the Makefile.am would conflict. > > > > The patch stack has been reshuffled and patches have heen merged, > > and two new ones on top have been added. > > Thanks, Dominik. This applies cleanly now. Nice. > I've also added an OVERVIEW > section to fvwm3all.adoc explaining how the man page is split up into > different sections. Shoudn't that go to fvwm3.1 as well? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: parser branch
On Wed, Nov 17, 2021 at 06:20:05PM +, Thomas Adam wrote: > On Wed, Nov 17, 2021 at 01:47:05PM +, Thomas Adam wrote: > > On Wed, Nov 17, 2021 at 02:38:19PM +0100, Dominik Vogt wrote: > > > I'd like to finish the parser work started in 2014. Is the old > > > branch still available somewhere? > > > > Remind me what work that was... > > I remember now. It came in two parts. The first part, you and I worked on > documenting the commands in BNF notation (back in 2016). That work is what > got carried across to fvwm. > > Then you created some experimental code to start to clean things up. > > That work was done in the mvwm repository [1], and the two branches you'll > want to look at are: Thanks, found them and rebased the patches to master. Couple of hours to fix conflicts. If you don't read from me for a while, that will be because fvwm crashes with the changed parser. ;-) > I can't really remember where we got to with that work either -- I'm hoping > this trip down memory lane might jog your memory more than it has mine. ;) The whole point in the completed part of the work was to make the command line parser pluggable. As far as I remember, that part was done, but needed more testing by users. Now that the rebase potentially contains some mistakes and may miss some things that have changed in between, it's even more important. There is a slight incompatibility of the new code with some thiung about quoting plus variable expansion. I had started a thread on the mailing list and asked if that would annoy anybody and the people who replied said no. Anyway (a) I cannot remember what it was exactly (we can dig it up from the mail archive) and (b) there won't be any progress if we make a new parser compatible with the crappy bugs of the old one. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (6) Man page changes -- second attempt
On Tue, Nov 16, 2021 at 01:36:53AM +0100, Dominik Vogt wrote:
> This is the full set of patches for splitting the man page, to be
> applied to master.
Second attempt. The style docs are not moved aound in the man
page. The .section files have been renamed to .ad because
asciidoc only evaluates "ifdef" in included files if they have a
known extesion. ".adoc" cannot be used because the pattern rules
in the Makefile.am would conflict.
The patch stack has been reshuffled and patches have heen merged,
and two new ones on top have been added.
0001: Man page split (only one patch now)
0002,3,4: General cleanup in man page.
0005: (new) Remove "open look" and "xview" from man page.
0006: (new) Remove "GlobalOpts" from man page, code and oteher files.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From de749130845071272c41c34e84de2ebd8e6dc61d Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Mon, 15 Nov 2021 16:52:38 +0100
Subject: [PATCH 1/6] Split main man page.
fvwm3.1 basic documentation
fvwm3commands.1 command documentation
fvwm3all.1 all documentation
fvwm3menus.1 menu documentation
---
doc/.gitignore| 1 +
doc/{modules => }/FvwmAnimate.adoc| 0
doc/{modules => }/FvwmAuto.adoc | 0
doc/{modules => }/FvwmBacker.adoc | 0
doc/{modules => }/FvwmButtons.adoc| 0
doc/{bin => }/FvwmCommand.adoc| 0
doc/{modules => }/FvwmConsole.adoc| 0
doc/{modules => }/FvwmEvent.adoc | 0
doc/{modules => }/FvwmForm.adoc | 0
doc/{modules => }/FvwmIconMan.adoc| 0
doc/{modules => }/FvwmIdent.adoc | 0
doc/{modules => }/FvwmMFL.adoc| 0
doc/{modules => }/FvwmPager.adoc | 0
doc/{modules => }/FvwmPerl.adoc | 0
doc/{bin => }/FvwmPrompt.adoc | 0
doc/{modules => }/FvwmRearrange.adoc | 0
doc/{modules => }/FvwmScript.adoc | 0
doc/Makefile.am | 61 +-
doc/README| 8 -
doc/{bin => }/fvwm-config.adoc| 0
doc/{bin => }/fvwm-convert-2.6.adoc | 0
doc/{bin => }/fvwm-menu-desktop.adoc | 0
doc/{bin => }/fvwm-root.adoc | 0
doc/fvwm3.adoc| 7 +
.../fvwm3.adoc => fvwm3_manpage_source.adoc} | 545 +-
doc/fvwm3all.adoc | 7 +
doc/fvwm3commands.adoc| 7 +
doc/fvwm3menus.adoc | 7 +
doc/fvwm3styles.adoc | 7 +
29 files changed, 350 insertions(+), 300 deletions(-)
create mode 100644 doc/.gitignore
rename doc/{modules => }/FvwmAnimate.adoc (100%)
rename doc/{modules => }/FvwmAuto.adoc (100%)
rename doc/{modules => }/FvwmBacker.adoc (100%)
rename doc/{modules => }/FvwmButtons.adoc (100%)
rename doc/{bin => }/FvwmCommand.adoc (100%)
rename doc/{modules => }/FvwmConsole.adoc (100%)
rename doc/{modules => }/FvwmEvent.adoc (100%)
rename doc/{modules => }/FvwmForm.adoc (100%)
rename doc/{modules => }/FvwmIconMan.adoc (100%)
rename doc/{modules => }/FvwmIdent.adoc (100%)
rename doc/{modules => }/FvwmMFL.adoc (100%)
rename doc/{modules => }/FvwmPager.adoc (100%)
rename doc/{modules => }/FvwmPerl.adoc (100%)
rename doc/{bin => }/FvwmPrompt.adoc (100%)
rename doc/{modules => }/FvwmRearrange.adoc (100%)
rename doc/{modules => }/FvwmScript.adoc (100%)
rename doc/{bin => }/fvwm-config.adoc (100%)
rename doc/{bin => }/fvwm-convert-2.6.adoc (100%)
rename doc/{bin => }/fvwm-menu-desktop.adoc (100%)
rename doc/{bin => }/fvwm-root.adoc (100%)
create mode 100644 doc/fvwm3.adoc
rename doc/{fvwm3/fvwm3.adoc => fvwm3_manpage_source.adoc} (99%)
create mode 100644 doc/fvwm3all.adoc
create mode 100644 doc/fvwm3commands.adoc
create mode 100644 doc/fvwm3menus.adoc
create mode 100644 doc/fvwm3styles.adoc
diff --git a/doc/.gitignore b/doc/.gitignore
new file mode 100644
index ..38ff07fe
--- /dev/null
+++ b/doc/.gitignore
@@ -0,0 +1 @@
+/*.ad
diff --git a/doc/modules/FvwmAnimate.adoc b/doc/FvwmAnimate.adoc
similarity index 100%
rename from doc/modules/FvwmAnimate.adoc
rename to doc/FvwmAnimate.adoc
diff --git a/doc/modules/FvwmAuto.adoc b/doc/FvwmAuto.adoc
similarity index 100%
rename from doc/modules/FvwmAuto.adoc
rename to doc/FvwmAuto.adoc
diff --git a/doc/modules/FvwmBacker.adoc b/doc/FvwmBacker.adoc
similarity index 100%
rename from doc/modules/FvwmBacker.adoc
rename to doc/FvwmBacker.adoc
diff --git a/doc/modules/FvwmButtons.adoc b/doc/FvwmButtons.adoc
similarity index 100%
rename from doc/modules/FvwmButtons.adoc
rename to doc/FvwmButtons.adoc
diff --git a/doc/bin/FvwmCommand.adoc b/doc/FvwmComman
parser branch
I'd like to finish the parser work started in 2014. Is the old branch still available somewhere? Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (5) Man page changes
On Wed, Nov 17, 2021 at 05:59:49AM +, Thomas Adam wrote: > On Tue, Nov 16, 2021 at 01:36:53AM +0100, Dominik Vogt wrote: > > This is the full set of patches for splitting the man page, to be > > applied to master. > > > > 1, 2 and 4 are unrelated cleanups. > > 3 and 5 implement the split. > > > > 4 conflicts with both, 3 and 5, so it can't be pulled out of the > > sequence. > > I'm not able to apply these patches cleanly. Patch 5 is failing to apply: > > .git/rebase-apply/patch:6588: space before tab in indent. > StaysOnTop, WindowListSkip > .git/rebase-apply/patch:6591: space before tab in indent. > WindowListSkip > error: patch failed: doc/fvwm3_manpage_source.adoc:5186 Please apply them now while I see what can be done to fix #5. Can you give me the hook scripts that do these checks so that I catch these things locally before sending patches? > Any thoughts? Some of the examples are indented with tabs; something like this: --snip-- Examples: ``` foo bar baz ``` --snip-- I moved around a big chunk of text because there was some problem with nested ifdefs. If this can be solved it's not necessary to shuffle text blocks. Let me take a look. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) fvwm3styles.1.
On Tue, Nov 16, 2021 at 08:28:12AM +, Thomas Adam wrote: > On Tue, Nov 16, 2021 at 01:18:24AM +0100, Dominik Vogt wrote: > > Two more patches for the man page branch, this time taking care of > > the style commands. > > > > 0001: General cleanup. > > 0002: Split of fvwm3styles.1. > > > > Need to be applied in this order. > > > > Please take a good look at the result of the patch stack. I think > > this is all going in the right direction. If you agree and see no > > problems, I'll make a fresh stack of acleaned up patches that can > > go to the master branch. > > I agree -- it's looking really good. :) The final patches are in the other thread. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) Split off menu documentation to fvwm3menus.1.
On Tue, Nov 16, 2021 at 12:50:29AM +, Thomas Adam wrote: > For the current documentation work you're sending patches for, I'm doing the > same thing, but I presume if you're squashing or editing changes together, it > makes it harder for me to know what I'm supposed to be tracking, that's all. Actually, I didn't edit patches until half an hour ago, and I'm not sure why you got a conflict. Well, I had two more unrelated patches at the bottom of the stack, and for the style manpage I moved roughly 2000 lines around in the source file. I have just a local master with a pile of patches, multiple topics in the same branch. The branch is rebased to origin/master once in a while, and certainly before sending patches. If you get conflicts, just ask for new patches. It's not worth the effort that you try to repair anything. Things will get much simpler with the big man page restructuring done. In any case, it's a good if another person looks at patches, not just me. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) Split off menu documentation to fvwm3menus.1.
On Tue, Nov 16, 2021 at 12:26:45AM +, Thomas Adam wrote: > On Tue, Nov 16, 2021 at 01:13:24AM +0100, Dominik Vogt wrote: > > On Tue, Nov 16, 2021 at 12:09:41AM +, Thomas Adam wrote: > > > On Mon, Nov 15, 2021 at 11:57:54PM +0100, Dominik Vogt wrote: > > > > On Mon, Nov 15, 2021 at 11:38:36PM +0100, Dominik Vogt wrote: > > > > > 0001: Some man page cleanup. > > > > > 0002: New man page fvwm3menus.1 > > > > > > > > 0003: Fix list formatting (attached). > > > > > > I've applied these three patches now, thanks. > > > > > > They still did not apply cleanly on top of ta/dv-manpage-sections, so I > > > had to > > > manually modify a few sections with the .rej file. > > > > > > Can you check I've not mangled anything? I do note some formatting > > > inconsistencies when viewing fvwm3menus.1 but I don't think that's > > > related to > > > any changes made so far. > > > > Just ignore it, I'll make a cleaned up series later, so we can > > dump the branch. > > That's fine. I'm using that branch to collect all the documentation changes > you're sending through as it's easier to manage -- so if those patches could > be based from that branch, it would certainly help. :) Sorry, won't work, I've already reordered, merged and edited patches. I don't want to commit a pile of junk like in CVS times. With Git I want much higher patch quality. :) Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) fvwm3styles.1.
On Tue, Nov 16, 2021 at 01:18:24AM +0100, Dominik Vogt wrote: > Off-topic: Can we remove the "globalopts" command description? And while we're at it, remove its implementation as well? At the moment, GlobalOpts: (1) Emits a deprecation warning ans prints the command that has to be used instead. (2) Executes that command automatically. We could remove just (2), leaving the warning in place, or just eliminate the command completely. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) Split off menu documentation to fvwm3menus.1.
On Tue, Nov 16, 2021 at 12:09:41AM +, Thomas Adam wrote: > On Mon, Nov 15, 2021 at 11:57:54PM +0100, Dominik Vogt wrote: > > On Mon, Nov 15, 2021 at 11:38:36PM +0100, Dominik Vogt wrote: > > > 0001: Some man page cleanup. > > > 0002: New man page fvwm3menus.1 > > > > 0003: Fix list formatting (attached). > > I've applied these three patches now, thanks. > > They still did not apply cleanly on top of ta/dv-manpage-sections, so I had to > manually modify a few sections with the .rej file. > > Can you check I've not mangled anything? I do note some formatting > inconsistencies when viewing fvwm3menus.1 but I don't think that's related to > any changes made so far. Just ignore it, I'll make a cleaned up series later, so we can dump the branch. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [PATCH] (2) Split off menu documentation to fvwm3menus.1.
On Mon, Nov 15, 2021 at 11:38:36PM +0100, Dominik Vogt wrote: > 0001: Some man page cleanup. > 0002: New man page fvwm3menus.1 0003: Fix list formatting (attached). Ciao Dominik ^_^ ^_^ -- Dominik Vogt From c23033b58a26b53da0a743c26d8f017719a1d5ac Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Mon, 15 Nov 2021 23:56:39 +0100 Subject: [PATCH 3/3] Fix list formatting. --- doc/fvwm3_manpage_source.adoc | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/doc/fvwm3_manpage_source.adoc b/doc/fvwm3_manpage_source.adoc index ac9d46be..d2bccf28 100644 --- a/doc/fvwm3_manpage_source.adoc +++ b/doc/fvwm3_manpage_source.adoc @@ -2865,44 +2865,37 @@ area. The string consists of spaces, characters and formatting directives beginning with '%'. Any invalid characters and formatting directives are silently ignored: + +-- *%l*, *%c* and *%r*::: Insert the next item label. Up to three labels can be used. The item column is left-aligned (*%l*), centered (*%c*) or right-aligned (*%r*). -+ *%i*::: Inserts the mini icon. -+ *%>* and *%<*::: Insert the sub menu triangle pointing either to the right (*%>*) or to the left (*%<*). -+ *%|*::: The first *%|* denotes the beginning of the area that is highlighted either with a background color or a relief (or both). The second *%|* marks the end of this area. *%|* can be used up to twice in the string. If you do not add one or both of them, fvwm sets the margins to the margins of the whole item (not counting the side picture). -+ *%s*::: Places the side picture either at the beginning or the end of the menu. This directive may be used only once and only as the first or last in the format string. If the *%s* is not at the beginning of the string, menus are not drawn properly. - -+ *Space*, *Tab*, *%Space* and *%Tab*::: Add gap of one space, or a tab, using the width of the menu font. When using a tab, the size of the gap can be one to 8 spaces since the tab position is a multiple of 8 from the edge of the menu. The whole string must be quoted if spaces or tabs are used. -+ *%p*::: Like Space and Tab *%p* inserts an empty area into the item, but with better control of its size (see below). - -.:: - +-- ++ You can define an additional space before and after each of the objects like this + -- 2.30.2
[PATCH] (2) Split off menu documentation to fvwm3menus.1.
0001: Some man page cleanup. 0002: New man page fvwm3menus.1 Note that there's a problem with formatting. When a paragraph contains a list like this: --snip-- == section blabla + item1:: some text + item2:: more text more paragraph text <- --snip-- The ending text goes back to the indentation of the section heading, or ig "+" is used it's indented like the list text. The workaround is to start a new list with smaller indentation with --snip-- .:: --snip-- But that creates an additional "." heading. See line 2904. Ciao Dominik ^_^ ^_^ -- Dominik Vogt From 990164dc07410d7c5920094b5634219bb3716608 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Mon, 15 Nov 2021 23:29:24 +0100 Subject: [PATCH 1/2] Cleanup man pge. --- doc/fvwm3_manpage_source.adoc | 56 +-- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/doc/fvwm3_manpage_source.adoc b/doc/fvwm3_manpage_source.adoc index 71e9c47a..b1d93c46 100644 --- a/doc/fvwm3_manpage_source.adoc +++ b/doc/fvwm3_manpage_source.adoc @@ -616,7 +616,7 @@ might look something like this: By default, SVG images are rendered as the image creator intended them to. But since SVG is a vector graphics format, the images can be -rendered at any chosen size and rotation, e.g. making it possible to use +rendered at any chosen size and rotation, e.g. making it possible to use the same icon file rendered at different sizes for the _Icon_ and _MiniIcon_ styles. @@ -1281,7 +1281,7 @@ desk, stickiness, shadiness, maximizedness, iconifiedness for all windows. Furthermore, some global state is saved. Fvwm doesn't save any information regarding styles, decors, functions or -menus. If you change any of these resources during a session (e.g. by +menus. If you change any of these resources during a session (e.g. by issuing *Style* commands or by using various modules), these changes are lost after saving and restarting the session. To become permanent, such changes have to be added to the configuration file. @@ -1554,7 +1554,7 @@ $[page.nx] $[page.ny]:: equivalent to _area_ in the GNOME terminology. $[w.id]:: - The window-id (expressed in hex, e.g. 0x10023c) of the window the + The window-id (expressed in hex, e.g. 0x10023c) of the window the command was called for or "$[w.id]" if no window is associated with the command. @@ -1729,7 +1729,7 @@ $[infostore._key_]:: $[...]:: If the string within the braces is neither of the above, fvwm tries to find an environment variable with this name and replaces its value if - one is found (e.g. "$[PAGER]" could be replaced by "more"). Otherwise + one is found (e.g. "$[PAGER]" could be replaced by "more"). Otherwise the string is left as is. + Some examples can be found in the description of the *AddToFunc* @@ -2359,17 +2359,17 @@ _Context_::: the current window, menu or icon. + _This_::: - whatever widget the pointer is on (e.g. a corner of a window or the + whatever widget the pointer is on (e.g. a corner of a window or the root window). + _Rectangle_ <__geometry__>::: the rectangle defined by <__geometry__> in X geometry format. Width and height default to 1 if omitted. - -If the context-rectangle is omitted or illegal (e.g. "item" on a ++ +If the context-rectangle is omitted or invalid (e.g. "item" on a window), "Mouse" is the default. Note that not all of these make sense -under all circumstances (e.g. "Icon" if the pointer is on a menu). - +under all circumstances (e.g. "Icon" if the pointer is on a menu). ++ The offset values _x_ and _y_ specify how far the menu is moved from its default position. By default, the numeric value given is interpreted as a percentage of the context rectangle's width (height), @@ -2464,7 +2464,7 @@ other position arguments. Sets a new menu style or changes a previously defined style. The _stylename_ is the style name; if it contains spaces or tabs it has to be quoted. The name "*" is reserved for the default menu style. The -default menu style is used for every menu-like object (e.g. the window +default menu style is used for every menu-like object (e.g. the window created by the *WindowList* command) that had not be assigned a style using the *ChangeMenuStyle*. See also *DestroyMenuStyle*. When using monochrome color options are ignored. @@ -2536,8 +2536,8 @@ SubmenusRight, BorderWidth 2, UniqueHotkeyActivatesImmediate, _BorderWidth_ takes the thickness of the border around the menus in pixels. It may be zero to 50 pixels. The default is 2. Using an -illegal value reverts the border width to the default. - +invalid value reverts the border width to the default. ++ _Foreground_ and _Background_ may have a color name as an argument. This color is used for menu text or the menu's background. You can omit the color name to rese
Re: [RFC] Splitting the man page
On Mon, Nov 15, 2021 at 08:53:48PM +, Thomas Adam wrote: > On Mon, Nov 15, 2021 at 09:00:43PM +0100, Dominik Vogt wrote: > > Now, about the contents of the new man pages: Splitting has not > > bought us that much yet. 75% of the contents are now in the > > commands man page. It's difficult to remove more things from that > > because the command list also contains general topics like menus, > > session management and coloursets. Moving these parts to > > different pages would also remove them from the command list. The > > original man page just has a bad structure. > > I suppose another way of looking at it might be to trying to convey things in > terms of functional areas, almost looking at things that a user might want as > they're configuring fvwm. Yes. An obvious candidate is a separate page for the menu related configuration and commands. That's about 1300 lines of 9500 - a good sized chunk. Menus have little in common with the rest of the core. >Right now, we're trying to lump together a lot of > similar things. We could go further and have something like: > > * An overview section explaining the anatomy/role of the window manager, > explaining things such as: > > - Windows (decorations, anatomy, etc.) > - Menus > - Core functionality > - Extending functionality (modules) Another good candidate, but may require some restructuring. > > It's not obvious how to move the Style command (a subsection of > > the commands page) to a different file without cluttering the > > source with ifdefs. > > That's a small price to pay for trying to increase readability, IMO. I'm reluctant to do this. While the "BEGIN/END" comments can potentially overlap, ifdef can not. Let's do some simpler changes first. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[PATCH] Fix resize calculations.
The earlier patch broke resize calculations by making windows too
big. This patch fixes this.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From f95313370fd977f95a02148cb0ab75d4230e86b7 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Mon, 15 Nov 2021 21:35:53 +0100
Subject: [PATCH] Fix resize calculations broken in earlier patch.
---
fvwm/move_resize.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fvwm/move_resize.c b/fvwm/move_resize.c
index 3e09666b..7091a984 100644
--- a/fvwm/move_resize.c
+++ b/fvwm/move_resize.c
@@ -856,12 +856,15 @@ static int ParseOneResizeArgument(
}
else if (sscanf(arg,"-%d",) == 1)
{
- size_change = scr_size - (int)(val * factor + 0.5) + add_size;
+ size_change =
+ scr_size - (int)(val * factor + 0.5) + add_size -
+ *ret_size;
}
else if (sscanf(arg,"+%d",) == 1 || sscanf(arg,"%d",) == 1)
{
size_change =
- (int)(val * factor + 0.5) + add_size + add_base_size;
+ (int)(val * factor + 0.5) + add_size + add_base_size -
+ *ret_size;
}
else
{
--
2.30.2
Re: [RFC] Splitting the man page
On Mon, Nov 15, 2021 at 06:42:20PM +, Thomas Adam wrote: > On Mon, Nov 15, 2021 at 07:04:57PM +0100, Dominik Vogt wrote: > > Guessed so. I believe automake + autoconf should support that > > automatically in the generated rules, but it needs to be tested. > > Anybody here who knows how that works? > > You don't get it for free, which is why you need the custom install/uninstall > targets: > > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Transformation-Rules.html#Transformation-Rules Yes, you don't get it free from autoconf, but automake should do it(?) You can even prefix the man page rules with "notrans_" to prevent transformation, but I don't know what this is all about. I'd expect that transformation is done at install time, not at build time? Anyway, someone who knows what is necessary needs to test it. -- Now, about the contents of the new man pages: Splitting has not bought us that much yet. 75% of the contents are now in the commands man page. It's difficult to remove more things from that because the command list also contains general topics like menus, session management and coloursets. Moving these parts to different pages would also remove them from the command list. The original man page just has a bad structure. It's not obvious how to move the Style command (a subsection of the commands page) to a different file without cluttering the source with ifdefs. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
[RFC] Splitting the man page
As discussed; the attached patch is a draft of how this could work.
1) It was necessary to remove the subdirs below doc/. All sources
and generated files are now in doc/. This also makes it much
easier to generate and install files. Makefiles don't work well
if files are located in different diectories anyway.
2) The Makefile.am now uses the standard "man1_MANS" file type to
describe the generated files. That should make local installation
rules unnecessary. Not sure whether it also handles the
"transform" stuff or not. For now I've removed it.
3) There's a new, experimental page "fvwm3commands.adoc" which
has a header and then just includes the file
fvwm3_commands.section. This file is generated from fvwm3.adoc by
extracting all text between the two comments
// BEGIN 'commands'
...
// END 'commands'
Sections to extract are listed in the variable "EXTRACT_SECIONS".
For now, all man pages simply depend on all section files. To
build real dependencies we'd have to extract the include
directives from the .adoc files, but this seems to be overkill.
The split man page sources are very simple:
--snip--
= FVWM3COMMANDS(1)
== NAME
fvwm3commands - fvwm build-in commands
include::fvwm3_commands.section[]
--snip--
4) Also removed the "QUIET_ASCIIDOC" thing because it seems to be
pointless to suppress single-line rules in the output. (Don't
care much about it, though.)
5) fvwm3.1 still contains everything. I'd like to rename that to
fvwm3all.1 and then find a way to create fvwm3.1 without the
sections that have their own page, containing placeholders that
point to the other pages.
6) We want to add a footer that points to the related pages.
--
Seems to do what is intended; "make install" and "make uninstall"
work fine, but I haven't tried to build and test a distro from
that.
Comments?
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
From 3994b8a87ac7d2637215dd514d21634e490f0a29 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Mon, 15 Nov 2021 16:52:38 +0100
Subject: [PATCH] DRAFT: Split main man page.
---
doc/.gitignore | 1 +
doc/{modules => }/FvwmAnimate.adoc | 0
doc/{modules => }/FvwmAuto.adoc | 0
doc/{modules => }/FvwmBacker.adoc| 0
doc/{modules => }/FvwmButtons.adoc | 0
doc/{bin => }/FvwmCommand.adoc | 0
doc/{modules => }/FvwmConsole.adoc | 0
doc/{modules => }/FvwmEvent.adoc | 0
doc/{modules => }/FvwmForm.adoc | 0
doc/{modules => }/FvwmIconMan.adoc | 0
doc/{modules => }/FvwmIdent.adoc | 0
doc/{modules => }/FvwmMFL.adoc | 0
doc/{modules => }/FvwmPager.adoc | 0
doc/{modules => }/FvwmPerl.adoc | 0
doc/{bin => }/FvwmPrompt.adoc| 0
doc/{modules => }/FvwmRearrange.adoc | 0
doc/{modules => }/FvwmScript.adoc| 0
doc/Makefile.am | 52
doc/README | 8 -
doc/{bin => }/fvwm-config.adoc | 0
doc/{bin => }/fvwm-convert-2.6.adoc | 0
doc/{bin => }/fvwm-menu-desktop.adoc | 0
doc/{bin => }/fvwm-root.adoc | 0
doc/{fvwm3 => }/fvwm3.adoc | 3 +-
doc/fvwm3commands.adoc | 7
25 files changed, 32 insertions(+), 39 deletions(-)
create mode 100644 doc/.gitignore
rename doc/{modules => }/FvwmAnimate.adoc (100%)
rename doc/{modules => }/FvwmAuto.adoc (100%)
rename doc/{modules => }/FvwmBacker.adoc (100%)
rename doc/{modules => }/FvwmButtons.adoc (100%)
rename doc/{bin => }/FvwmCommand.adoc (100%)
rename doc/{modules => }/FvwmConsole.adoc (100%)
rename doc/{modules => }/FvwmEvent.adoc (100%)
rename doc/{modules => }/FvwmForm.adoc (100%)
rename doc/{modules => }/FvwmIconMan.adoc (100%)
rename doc/{modules => }/FvwmIdent.adoc (100%)
rename doc/{modules => }/FvwmMFL.adoc (100%)
rename doc/{modules => }/FvwmPager.adoc (100%)
rename doc/{modules => }/FvwmPerl.adoc (100%)
rename doc/{bin => }/FvwmPrompt.adoc (100%)
rename doc/{modules => }/FvwmRearrange.adoc (100%)
rename doc/{modules => }/FvwmScript.adoc (100%)
rename doc/{bin => }/fvwm-config.adoc (100%)
rename doc/{bin => }/fvwm-convert-2.6.adoc (100%)
rename doc/{bin => }/fvwm-menu-desktop.adoc (100%)
rename doc/{bin => }/fvwm-root.adoc (100%)
rename doc/{fvwm3 => }/fvwm3.adoc (99%)
create mode 100644 doc/fvwm3commands.adoc
diff --git a/doc/.gitignore b/doc/.gitignore
new file mode 100644
index ..07576799
--- /dev/null
+++ b/doc/.gitignore
@@ -0,0 +1 @@
+/*.section
diff --git a/doc/modules/FvwmAnimate.adoc b/doc/FvwmAnimate.adoc
similarity index 100%
rename from doc/modules/FvwmAnimate.adoc
rename to doc/FvwmAnimate.adoc
diff --git a/doc/modules/FvwmAuto.adoc b/doc/FvwmAuto.adoc
similarity index 100%
rename from doc/modules/FvwmAuto.adoc
rename to doc/FvwmAuto.adoc
diff -
Re: [PATCH] (7) Miscellaneous cleanup
On Mon, Nov 15, 2021 at 11:45:58AM +, Thomas Adam wrote: > On Mon, Nov 15, 2021 at 12:11:51PM +0100, Dominik Vogt wrote: > > Thanks. Will apply. With respect this this patch: > > > 0007: Remove the "MWM COMPATIBILITY" section. Nobody cares anymore. > > Presumably the "OPEN LOOK AND XVIEW COMPATIBILITY" section can go as well? Yup, I can't even remember what that was. :-D Ciao Dominik ^_^ ^_^ -- Dominik Vogt
