Subject: Re: Xinerama merger -- part 1

From: "Dmitry Yu. Bolkhovityanov" <[email-masked]>
Date: Fri, 27 Jul 2001 21:35:49 +0700

On 27 Jul 01 at 11:56, [email-masked] wrote:

>  - Code is written with 80 characters per line, a basic
>    indentation width of 2.
>  - Always compile the code with and without your ifdefs.  Remove
>    all warnings (compile with 'make CFLAGS="-Wall -Werror -g").

    Okay.  I usually set CFLAGS="-W -Wall" -- this gives more interesting
info.

>  - Please keep in mind that that fvwm is controlled and configured
>    by configuration commands, not by environment variables.  It's
>    not necessary to make features configurable via the
>    environment. (I'll remove the corresponding code).

    As to this point, one reason was an ability to pass these parameters to
modules (so that they will automatically be inherited via environment), and
another is that some other proggies can theoritically use XiSupp.c (think
about XMMS and various toolkits) or a similar thing, and they have no
standard means of communicating to FVWM.

    Sure, modules interface can (and should?) be extended to accept
PrimaryScreen change, but this is mostly irrelevant -- primary screen is
usually taken into account only at startup.

>  - Please try to use one of the formatting styles that is used in
>    the fvwm code (indentation style).  Try not to put commands on
>    the same line as a condition.

    BTW, I looked for some style guide inside fvwm sources, but didn't find
any.  Your recommendations are definitely worth putting to some
fvwm_style_guide.txt.

>  - RandR support will certainly be no part of 2.4.1.  You should
>    really try to split the patches you made on your disk into
>    several patch files that can be applied individually.

    Well, the RandR support can be at the same position as multibyte support -
it is present but switched off by default.  When HAS_RANDR is undefined,
RandR will have no effect at all -- nothing will be queried in XiSupp.c, and
no RandR handlers will be called in events.c and FvwmPager.c.

    I added RandR support as an additional step forward -- it enables to
understand which currently used concepts will become obsolete in some near
future, and what should be changed for more flexibility.  That was the main
goal, not an ability to use resize feature of yet very rarely used KDrive.
;-)

> One question about the XiParseGeometry function:  I don't
> understand why is does so many things.  It really shouldn't do
> more than just parsing the geometry string like XParseGeometry
> does and use the same signature.  I'll comment out all the extra
> stuff for now.

    The answer is that modules do way too many calculations themselves
(negative handling, gravity selection, etc.).  Much of them are duplicates,
and with submitted XiParseGeometry() modules become much simpler -- you add
one/two lines and remove sometimes several dozens (look at patches for
modules).

    Well, maybe XiParseGeometry is not a very appropriate name, in fact what
this function does is very similar to XWMGeometry.

       ___________________________________________________________________
       Dmitry Yu. Bolkhovityanov  |  Novosibirsk, RUSSIA
       phone (383-2)-39-49-56     |  The Budker Institute of Nuclear Physics
                                  |  Lab. 5-13
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Sat, 28 Jul 2001 13:07:27 +0200

> standard means of communicating to FVWM.

Ah, okay.  The solution to both problems is the module interface.
Some settings like "Colorset" are communicated to the modules over
the module pipe.  The state of Xinerama{En,Dis}abled can be easily
communicated too.  In fact I have already done this.  Take a look
at the code in virtual.c:

  void CMD_XineramaDisable(F_CMD_ARGS)
  {
    XineramaSupportDisable();
    BroadcastConfigInfoString("XineramaDisable");
  }

Any module listening for M_CONFIG_INFO packets will receive the
string "XineramaDisable" when the function is called an can react
properly.  I think I still forgot to send this string when the
module is initially started, so the module's Xinerama state may
not match fvwm's state at first.  I will take care of this.

Any program that wants to communicate to fvwm tightly should be
written as an fvwm module.  Another way would be to communicate
via FvwmCommand, but that is inefficient and littl internal
information about fvwm's state it communicated this way.

> any.  Your recommendations are definitely worth putting to some

Another one of the things that nobody has the time and leisure to
do.  The common practice is that everybody formats his code pieces
as he likes.  Whenever one of the more active developers works a
lot with a certain piece of code (mostly me) he is free to
reformat that code as he likes.  I think there would be a lot of
potential for an extensive religios war about brace, whitepace and
comment placement if we really tried to write something down :-)

> goal, not an ability to use resize feature of yet very rarely used KDrive.

Let me explain why I commented it (actually, commenting it made a
huge, big mess of XineramaSupport.c):  The risk that new problems
are introduced in the *stable* 2.4.1 release has to be minimised.

> this function does is very similar to XWMGeometry.

Well, then let's split up the function.  XiParseGeometry() does
exactly what XParseGeometry() does but with the additional
"@<screen>" parsing.  Another function could take its output as
input and do the additional calculations.  Or if that won't work,
there should at least be an alias of the function that can be
called like XParseGeometry().

BTW, if there is any risk, that rewriting the modules' placement
code may screw things up, that part should wait until after 2.4.1.
Cleaning up the code ca not be a priority in stable releases.  Of
course you can still include the appropriate patches in your code,
but please comment them by ifdefs for now.

Bye

Dominik ^_^  ^_^

 --
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]