[Ulf Lamping <ulf.lamping@xxxxxx>]

Hi List, hi Ronnie!

Please find attached the second version of the "recent user's file" patch.

If you don't find any reasonable bugs in the code, could you please check it in,
as diffing the code is a bit complicated for me now (I have to edit the diff file
a lot by hand), as I have made more recent GUI changes in my sourcetree,
which I will send in the future, if they are finished.

I have no problem, applying changes to this again, to improve the "recent user file" code,
if someone suggesting improvements or moving parts of the source to a different
place (where it is more suitable).


----------
"Ronnie Sahlberg" <sahlberg@xxxxxxxxxxxxxxxx> schrieb am 07.09.02 15:58:31:
> Hi,
> 
> Can you update the patch so it applies to current CVS.

Well, that's some sort of a moving target. At the time I send the last patch, 
it WAS the current CVS version. The new patch should be diffed to the CVS today.

> Also, can you enhance the patch so that it also updates gtk2?

As I currently have no GTK2 port for win32, this is a problem for me,
as I cannot test or even compile this.

BTW: why do we forked the code for gtk and gtk2 at all? Are the differences that much?
In my experience, forking some code will lead to a lot of double work 
with usually no real benefit.

> 
> >-put repeatedly used submenu item always again at latest place, to get the
> right history order
> Just move it to the top everytime anything is accessed on the recent list.
> That should
> be good enough.
> I dont think it has to be too advanced like keeping track on how many times
> it has been
> used and do the sorting based on that.

Is implemented in this patch (found some time to do it).

> 
> 
> Hmmm.
> You call add_menu_recent_capture_file() from file.c .
> I dont think I like this very much.
> Would it be possible to move this call to somewhere inside gtk[2]/main.c
> instead?
> I think it would be way better to piggy back onto the file_open dialog box.
> 

I thought about that for a while. The thing is, if we would do it like you write,
and someone opened a file by using a command line option,
this would not be included in the recent capture file list, as I would expect.

BTW: Why do you think, it is not the right place for it? In file.c a lot of other gui
calling dependant code is placed there (e.g. the set_menus_for_capture_xy() functions).

> 
> The two filenames
> RECENT_KEY_CAPTURE_FILE  and RECENT_KEY_DISPLAY_FILTER both contains
> two dots in the filename. Could you change the names to something with just
> one dot or no dot at all?

Changed, in this patch, they are now "one-dotted", e.g. "recent.capture_file".
BTW: What is the general difference between the dots and underlines?

> 
> 
> There is an awful lot of code to implement this feature in the patch.
> Could you look at making it smaller/simpler?
> It feels like it it would become much smaller and simpler if you redid the
> implementation again.

Should be a bit better now (see below).

> 
> Perhaps as simple as:
> * read_recent_files()  which reads the recent file and populates the
> in-memory list and all menuitems.  Let the function start by destroying the
> existing in memory list and all existing menu items. Let the function ignore
> any duplicates when reading this file (so you dont have to worry about
> duplicates elsewhere , making life simpler)
> * write_recent_files() which just opens the file and overwrites it with the
> 10 first entries in the
> in-memory list. When it has written these 10 entries, just do a
> read_recent_files() before returning which makes the in-memory list and the
> menu reflect the newly written data automagically.
> * new_recent_file() which just adds a file to the start of the in-memory
> list (without redards or checks of if the file already exists in the list or
> not, we dont worry about duplicates) and then just calls write_recent_file()
> before it returns.  This updates the file and also automagically removes any
> duplicates.
> 
> Then you should only need to do read_recent_files() when ethereal starts and
> just do new_recent_file() whenever a user clicks the OK button on the file
> dialog nothing more.
> Very simple and very few lines to implement.
> 

Well, what you describe looks like a few lines of code. But what you will need is
nearly the same things I currently have implemented. Keep in mind that there are
two lists and a file to maintain.

I have no good feeling writing the recent file everytime applying a display filter.
This will lead to problems at least if the writing will fail.
As there is a missing feature now, telling the user by a simple_dialog, in case that the
writing of the user's recent file failed. This dialog would pop-up everytime, the
display filter is changed then (otherwise only at quitting Ethereal).

------
So, I reworked the code, collecting a lot of the code for maintaining
the recent capture filter list together in the add_menu_recent_capture_file_absolute()
function (this reduced the amount of code a lot for this).

Regards, ULFL
______________________________________________________________________________
Die clevere Geldreserve: der DiBa-Privatkredit. Funktioniert wie ein Dispo, 
ist aber viel gunstiger! Alle Infos: http://diba.web.de/?mc=021104

Attachment: recent2.zip
Description: Binary data