Re: X11 support

[Roland Illig <roland illig gmx de>]

Pavel Tsekov wrote:
> On Wed, 9 Feb 2005, Roland Illig wrote:
> > 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.

Hello Pavel,

first: Thanks for the review. As usual I tried to do something good, but 
I did not find the right arguments for it.

In this case, I wanted the GModule dynamic loading to be placed outside 
the key.c file, because the GModule part is not necessary to understand 
key handling. Only the X11 connection is.

> [...] 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.

That's what I'm trying to do since I've started programming the MC. But 
you're right that we should first have a debate about the overall 
organization of MC. I'll start one in a separate thread.

> 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.

Well, that's typically for me. I think I always design things one level 
above what is really necessary. Perhaps some other parts of mc might use 
the X11 functions (although I don't know which part would that be), so I 
provided the reference counting.

Maybe I can cut down the code to half its size, and we can agree, like I 
did with the code for displaying the device numbers. ;)

Roland