Re: [Nano-devel] search history

[Ken Tyler]

On Mon, 21 Oct 2002, David Benbennick wrote:

> On Mon, Oct 21, 2002 at 02:28:48PM +1000, Ken Tyler wrote:

> 1) If you're using gcc, I suggest turning on some extra warnings,
 
> -Wall -Wcast-qual -Wcast-align -Wwrite-strings -Wstrict-prototypes
> -Wmissing-prototypes -Wmissing-declarations -Winline -Wnested-externs

Will do.
 
> 2) Perhaps you could make search_history and replace_history be 
> pointers, i.e.
 
> historyheadtype *search_history;
> historyheadtype *replace_history;

I knew this would be a complaint. I can't see the point in allocating a
pointer, taking an address then using -> when all that's needed is a '.'.
Is it better to put a constant value into a pointer and use that rather
than the object itself ?

> All the other linked list structures in Nano work this way.

The design of the linked list (from an old Amiga book) is that all nodes
are identical with a pair of extra 'phantom' nodes, one at the begining
and one at the end of the list. This pair of 'phantom' nodes being
overlaid into one real header node but offset by one pointer size.  This
way nodes can be added and deleted without constantly checking for NULL
pointers. It might be common to handle lists this way, I don't know, but
it seems a clever way (not mine) todo it.

> 3) The first 8 lines of search.c:history_init() are pretty ugly.  Is it
> really necessary to assign a (historyheadtype **) value to a
> (historyheadtype *) variable?

Probably not, after a quick look it makes no sense.  historytype and
historyheadtype structures share some identical names and pointers, on
initializing but before the blank line node is added, search.history
contains two pointers to its own members and a null.  Thinking.

> 4) Take out the first hunk of the winio.c change.

Is that the $id line ?  Don't know what happened there.

> 5) Could you add a bit to global.c:thanks_for_all_the_fish() to deallocate 
> the history lists?

Briliant idea - an oversight of the first order!

> In general it looks great.  I'll start using it as soon as it works in
> Pico mode.  By the way, I think it's fine if you post the 6K gzipped
> version on nano-devel.

What I would prefer to do is get the (fixed up) current version into cvs,
let it have some use, wait till some consensus arises regarding what
default strings should be offered up and the pico/nano compatability
question before pico history is added.

There are several variables in use around here holding various old and new
search/replace strings, some could probably go in a rewrite.

Ken.