Le 10/04/2023 à 18:16, Quentin Barnes a écrit :
On Mon, Apr 10, 2023 at 12:31:49PM +0200, Maxime Soulé wrote:
Le 08/04/2023 à 22:31, Quentin Barnes a écrit :
On Sat, Apr 08, 2023 at 07:09:54PM +0200, Maxime Soulé wrote:
[...]
Another thing that comes in mind: don't forget that these new macros are
bound to Xrandr initial detection. If the layout has later been overridden
using MonitorLayout config variable, the macros won't reflect the user
layout.
This is what I mentioned about boundary cases in my initial post.
Being a noob to window manager code, I'm sure there are all sorts of
boundary cases I'm not aware of coming from existing ctwm features
and xrandr behavior.
One area I didn't test was what happened if ctwm isn't configured
with xrandr. Is any special handling needed?
If Xrandr is not available, or cannot be used because its version is too
low, or any other reason XrandrNewLayout
(https://github.com/fullermd/ctwm-mirror/blob/master/xrandr.c#L22)
returns NULL, the layout is initialized with root coordinates:
https://github.com/fullermd/ctwm-mirror/blob/master/ctwm_main.c#L455-L457
As far as I can tell, it looks like when there's no xrandr compiled
in, appropriate monitor values are just substituted, so things
should just work as-is, right?
I added a comment to your commit to handle this specific case, for which
there is no monitor names at all.
On redundant checks in my patch, after reading more code, it looks
like I could remove the "if (RLayoutNumMonitors(Scr->Layout) > 0) {
... }" check.
https://github.com/qbarnes/ctwm-mirror/blob/e462a9e8ad1677fce6f01176e006f07e9b37278b/parse_m4.c#L271
Prior to main() calling LoadTwmrc(), which in turn runs my patch,
the previous code in main() has already checked to ensure that
RLayoutNumMonitors(Scr->Layout) will return a value greater than 0,
correct?
Yes.
Just as a note, ctwm can be launched with --cfgchk and no X available,
to check its configuration and leave:
DISPLAY= ctwm --cfgchk .ctwmrc
for example. In this case, we fall into the case where Xrandr is not
used, so the layout defaults to root coordinates, so always one monitor.
I'm still not sure how "Scr->Layout" all works with multiple screens
and when it can change, so I had left it in there the patch when I
first coded it.
Once set at startup using Xrandr info, Scr->Layout can only change using
the MonitorLayout variable during the parsing of the configuration,
never after. Your code should be called before any MonitoLayout use, so
it is OK. As ctwm boots very quickly, it has been decided to restart it
when monitors layout changes instead of handling the change without
restarting it. It is a way easier to handle.
Perhaps XRANDR_ (or XRR_) prefix could eliminate a possible confusion? I
don't use MonitorLayout myself, but for those using it, it could be
misleading.
Ah! Excellent point! I hadn't thought about how the macros' values
are the ones that are returned by xrandr calls (or substituted values
if no xrandr), and not necessarily the monitors' values after being
fully configured by the user.
I think "XRR_" is a good prefix. With not abbreviating "MONITOR",
the macro names are already long enough!
I agree with XRR_ prefix, but I have no opinion about removing or not
the following MONITOR_ word.
++
Max.
