Re: [RFE][PATCH] Display free space on device in panels

[Pavel Tsekov <ptsekov gmx net>]

On Wed, 31 May 2006, Jindrich Novy wrote:

> On Tue, 2006-05-30 at 14:56 +0300, Pavel Tsekov wrote:
> > On Mon, 29 May 2006, Jindrich Novy wrote:
> >
> > > On Fri, 2006-05-19 at 14:29 -0400, Pavel Roskin wrote:
> > > > Can we avoid any highlighting?  I think Far Manager does it right:
> > > > http://red-bean.com/proski/mc/far.png
> > >
> > > Attaching the patch without highlighting. The main changes are that I
> > > moved the show_free_space() into main.c since screen.c lacks a header
> > > file and a call is needed from cmd.c to minimize frequency of gathering
> > > filesys info. The free space widget isn't displayed at all when browsing
> > > a non-local fs. Filesystem statistics are updated immediately when mc
> > > requests a reread (ctrl-r is pressed).
> > >
> > > Please review, some further mistakes are possible.
> >
> > Now, after I reviewed the patch for a second time I am pretty convinced
> > that the free space info should be displayed in the mini status area
> > and not on its upper frame i.e. as it is in FAR manager (what Pavel
> > Rosking suggested).
>
> Yes, otherwise it would be too little space for cwd, especially for
> 80x25 terminals.

Well, maybe I didn't explain well - I mean't the horizontal line between
the mini status and the directory listing.

> > MC tries to optimize the screen drawing by updating only certain parts
> > of the screen (the directory contents, the mini status) when not otherwise
> > necessary. Simply put MC doesn't update the whole screen when you go down
> > one file in the panel, when you change a directory, etc. With the current
> > patch you actually break that optimization by inserting a call to
> > mini_info_separator() in show_dir() which is called each time you move
> > through the file list for example.
>
> Ok, the only speed related optimization I see is to not to draw the free
> space widget every time the show_free_space() is called, but only in
> case when old_cwd and panel_cwd differs. The trick to force redraw when
> user presses ctrl-r by setting old_cwd to NULL will work then anyway.

Maybe this wasn't clear as well  - I'll try by pointing you directly to 
the code. See the difference between paint_panel ()  and 
panel_update_contents().

> > Your patch may be modified to so that it will work properly i.e. it wont
> > break the optimization but it becomes ugly (codewise). It may be modified
> > to be not so ugly by sacrificing the optimization a bit i.e. put
> > a call to mini_info_separator () in panel_update_contents(). It also
> > can be made to fit into the current scheme by making the free space info
> > part of the mini status area.
>
> Did you do some profiling that it slows mc down significantly? I haven't
> seen a noticable delay, especially after show_free_space() doesn't draw
> the widget each time it's called.

There is no need to profile MC. It is not speedup in terms of CPU cycles
but in reduced screen update i.e. why paint the frame each time if it
really doesn't change at all.

> > Now, there are some other small issues with the patch as-is:
> >
> > --- mc-4.6.1a/src/main.c.showfree	2006-05-29 12:41:36.000000000 +0200
> > +++ mc-4.6.1a/src/main.c	2006-05-29 13:04:50.000000000 +0200
> > @@ -402,6 +409,8 @@
> >       int reload_other = !(force_update & UP_ONLY_CURRENT);
> >       WPanel *panel;
> >
> > +    show_free_space(current_panel);
> > +
> >       update_one_panel (get_current_index (), force_update, current_file);
> >       if (reload_other)
> >   	update_one_panel (get_other_index (), force_update, UP_KEEPSEL);
> >
> > This part is not necessary. And it will draw the free_space_info() even
> > if there is no mini status (though you won't see it).
>
> show_free_space() checks whether free_space != 0 otherwise it won't draw
> anything.

Yes. But it will draw when the mini status is disabled. If you 
are not convinced - you could try stepping trough the body of 
update_panels() with a debugger. This slows the execution time and
it will allow you to see the free space info being printed.