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

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:

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

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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]