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:
[...]
> > The names have always been prefixed with an alpha characters and not
> > a simple digit string as far as I know, but I didn't know if that
> > was just a convention or a requirement.
>
> I misread, I thought you spoke about monitor names collision :)
>
> That said, I don't think a monitor name can begin with a digit. Names are
> built using the corresponding connector name as found in the driver, like
> here:
> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/drm_connector.c#L91-L110
Not only starting with a digit, but the entire string be nothing but
digits that could end up matching a simple integer.
Seems like a reason assertion here that Linux community wouldn't let
a connector be a simple string of digits. Double-underbar removed.
(Thanks for the kernel code pointer. GPU/DRM modules is not an area
I've trampled in before. I didn't even know the term "connector"!)
> > > > What do people think of this change overall? Was what it something
> > > > that makes sense and might be useful to others? Is there a better
> > > > or different way to do what I need to do instead?
> > >
> > > Out of curiosity, do you have some examples of use of these new macros?
> >
> > Sure! I can provide the examples of what I had previously mention > [snip]
>
> Thanks, it's interesting!
>
> 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?
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?
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?
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.
> 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!
> ++
>
> Max.
Quentin
