Re: X11 support

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Wed, 9 Feb 2005, Roland Illig wrote:

> Hi all,
>
> I have written a module to get the X11/GModule support out of src/key.c.
> Separate things---separate files.

Well, they aren't actually different things. The code that you refer to
deals with key modifiers when MC is running in the X environment. And the
code in key.c is the one interested in key modifiers no matter which
os/terminal emulator/desktop envirnoment combination MC is running in.

> With this module src/key.c can be simplified. I also added the framework
> for defining a custom error handler. This should be a good step on the
> way to eliminate the "openssh bug".

I don't think that you actually simplify key.c. The bulk of the code, say,
in get_modifier is QNX specific and this code is still there. Ok, if you
split the body of get_modifier() into separate functions it would be
more likely to convince me that you are actually trying to simplify
and deobfuscate the code. If this is the beginning of a planned effort to
cleanup MC's code, make it clear and so on and this is the first patch of
many to come - good, but isn't it better first discuss such an effort, set
the milestones and then act accordingly.

Now back to the patch - I don't really think that we would really need
mc_XOpenDisplay, mc_XCloseDisplay and mc_XQueryPointer more than once
and they don't seem like obvious candidates to have external linkage.
If you choose to go your way I'd suggest to implement the code which
queries the modifiers in a seaparate file say x11key.c or something like
that and then call it in the proper place and that's it. I also want to
address the reference counting that you've implemented - do we really need
this ? It is just a simple task that you have to solve. Also mc_x11_done()
seems to do more work than required since you actually increment
'usage_count' only once.