Re: New "Multiscreen" megapatch

[Pavel Roskin <proski gnu org>]

Hi, Walery!

I just want to acknowledge reciving this patch.  I really like that you
documented it very well.  I haven't read the patch yet, it will probably
be a separate message.

> This patch (mc-multiscreen-patch) adds possibility to
> open multiple internal viewers and internal editors at once.
> This fearure is almost the same as in the FAR manager.
> I'm sure it is will be very convenient for developers,
> but not only.

I discussed this feature of Far with Miguel and he supported this idea.
It's great that we have an implementation now.

> When you have at least one open editor or viewer,
> you can use the following keys:
>
> F12: Call "dialog list" to choose dialog to switch to.
>      Just like FAR manager.
> Alt-U: Switch to previous dialog
> Alt-I: Switch to next dialog

Sounds good.

> When you exit from MC when other editors and/or viewers
> are sleeping in the background,
> all viewers and unmodified editors are killed,
> but modified editors are activated and user have exit
> from this editors manually with or without saving a file.

Unfortunately, there is a hex editor in the viewer.  But we probably can
ignore this problem for now.  The hex editor is a hack and should not
stand in our way.

> Implementing this feature was impossible
> without special hacks on MC dialog management system.
> With old system ALL dialogs HAVE TO BE modal,
> so you couldn't switch between whem randomly.
> But now it is fixed.

Great!

> Almost ALL new or changed code in enclosed into the
> #ifdef HAVE_DLGSWITCH
> #endif
> pair. HAVE_DLGSWITCH can be defined in config.h.

Please use comments after #endif.  Use /* HAVE_DLGSWITCH */ is the
preceding code is used when HAVE_DLGSWITCH is defined.  Otherwise use
/* !HAVE_DLGSWITCH */

It's easier to read code with such comments.

> So it is (probably) safe to apply to current sources,
> because it is easy to turn off the feature.

Yes.  Some features affect portability (e.g. subshell support which
doesn't work on QNX Neutrino), but your patch should be safer in the long
run, so at some point all those directives will be hopefully removed (when
the new code works well).  I see no reasons to keep more configure options
just for fun.

> Only some peaces of code is not enclosed in
> #ifdef HAVE_DLGSWITCH .. #endif pair.

That's Ok.

> For example it is a new glib-based logging handler
> in main.c. I thinks it will be useful for debugging
> various peaces of MC anyway.

Great!  I was thinking about implementing it!

> Probably we can make this feature "experimental"
> and provide a ./configure switch to turn it on,
> for example "--enable-multiscreen" or
> "--enable-dlgswitch".

I'll check the patch to see if this precaution is necessary.

> About debugging.
>
> The patch extensively uses glib-based logging for
> debugging purposes. To turn this logging on you have
> to #define MY_DEBUG in config.h.

MC_DEBUG sounds better.

> The handler writes all logging messages to ~/.mc/debug.log.
> Without #define MY_DEBUG only fatal errors is handled:
> an error message is printed on the screen
> and program aborts with a core dump.

That's also a good idea.  There are some fatal cases in VFS that should be
handled in such way.  It's important to ask the user to write to the list
what he or she was doing before the crash happened.

> IMPORTANT NOTE!
>
> The patch is big and complex
> (that's why I've called it "megapatch"),
> so it should be applied AFTER the new release,
> but BEFORE cleaning from GNOME.

I was going to tell you exactly that!  I appreciate your responsive
approach.

> If we clean from GNOME before applying the patch
> it would be wery difficult to apply it
> cause HUGE changes in the source core.

Ok.

Thank you!

-- 
Regards,
Pavel Roskin