Re: [patch] Synchronization of the panels

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Sat, 2 Apr 2005, Siver Andrey wrote:

> I've rewriten my previous patch (main.c.diff, sync.inc):
> 1) Now synchronization is doing by time (SYNC_DELAY keeps inverval in seconds; 2 secs by default);
> 2) The synchronization is loosing for some DLG_ signals for efficiency;
> 3) Add some errors checks.
>
> It looks a little more stable and faster.
>
> But there is some MC behaviour that I could not understand or explain
> now. After some numbers of the update calls (3-5 and more), "enter" key
> does not work any longer for any directory of the panel opposited to the
> updating: error occurs: "cannot change directory". 'magic_path' function
> does not detect any problem though. I do not know whether it due to my
> patch, or due to MC or system property.
> Also it looks like my patch behaviour depends on type of terminal...
> Please test it.
>
> For those who possesses configure magic I also prepared patches for
> main.h and configure.ac (I also registered "enable-synchro" option; it

The configure patch is pretty simplistic - much more have to be done. I'll
try to help with that.

> will be useful until patch become stable); you may use sync4.h and
> sync4.c also. Unfortunately I'm not such a person now :).

Please, do not send two different sets of files for the same thing i.e.
sync4.* and sync.inc. You should switch to sync4.c and sync4.h.

> Suggestions are welcome.

Ok - several points:


1) Have you tested your patch with `auto_menu' set to 1 ? From what I see
   by reading your patch you're forcing the code to send DLG_IDLE here:

    if (msg != DLG_IDLE && msg != DLG_INIT) {
        sync_need_panels_redraw = 0;
        set_idle_proc (h, 1);
    }

   This could be pretty nasty with the current processing code for
   DLG_INIT in `midnight_callback ()'.

   You can change the default value by editing the value of `auto_menu' in
   ~/.mc/ini to 1.

2) Your code will call multiple times `sync_setup_panels()' from within
   the DLG_IDLE handler in `midnight_callback()'. On systems which do not
   support dnotify it could be useful to have a flag which will indicate
   that dnotify is not present so `sync_setup_panels()' should do nothing
   after the first unsuccesful call i.e. if the following call

     fcntl(sync_fd[panel_id], F_NOTIFY, DN_MODIFY
                                | DN_CREATE
                                | DN_DELETE
                                | DN_RENAME
                                | DN_MULTISHOT
                        );

   fails and `errno' is set to EINVAL you can set a flag and do not
   call `sync_setup_panels ()' anymore for obvious reasons. Same for
   `loose_sync ()'.

3) You are doing a quite obscure error handling through the `sync_err'
   pointer. Also why do you set the different bits in in `sync_err' if you
   never check them ?

4) There seems to be no cleanup function which closes the open
   directory filedescriptors. Well you close them before opening them
   in `_setup_sync'. But a general cleanup should be available
   to be called on MC's termination or something like that.

5) Please, declare everything that you are going to use outside of the
   sync4 module in sync4.h . Also your changes to main.h should be
   actually put in sync4.h. Every function or variable that is used in
   sync4.c should be made static.

6) Avoid writing code like this

     if (!(sync_errs[0]||sync_errs[1])) add_hook(&idle_hook,update_hook,&sync_update_flag); /* hook for synchronization */

   It should be written like this


     /* hook for synchronization */
     if (!(sync_errs[0]||sync_errs[1]))
         add_hook(&idle_hook,update_hook,&sync_update_flag);

   You should try to arrange your code as the rest of MC's code.

6) You should include config.h at the beginning of sync4.c like this:

     #ifdef HAVE_CONFIG_H
     #include "config.h"
     #endif

I think this is enough for now.