Here are some doubts and questions I had according jmb's suggestions:
>> + /* TODO: This would be best taken care of in the front end specific
>> code
>> + as it seems to be necessary on risc os only */
>> + /* if (global_history_recent_count == 1)
>> + ro_gui_window_prepare_navigate_all(); */
>
> Probably. However, the frontend needs some way of being notified about
> this. I'm not entirely sure why this exists here, btw.
Would a #ifdef riscos be OK here? I don't know if there is any sense
in informing all the frontends about this event if only RISC OS needs
this.
Also, the GTK front end doesn't use the recent URLs code at all as it
loads all urls from urldb for completion. I think it would be best to
use #ifnfdef for the GTK frontend as now the function produces a
warning.
>> Index: desktop/history_global_core.h
>> ===================================================================
>> --- /dev/null 2009-04-16 19:17:07.000000000 +0100
>> +++ desktop/history_global_core.h 2009-07-01 14:22:40.000000000 +0100
>> +
>> +extern struct tree *global_history_tree;
>
> What requires access to this? Might it be better to hide it and provide
> accessors/mutators for the things that are needed?
This has to be passed to the front end treeview code. The tree struct
implementation is already hidden so I'm not sure what the
accessors/mutators would have to deal with :)
>> +#define TREE_ELEMENT_URL 0x01
>> +#define TREE_ELEMENT_LAST_VISIT 0x02
>> +#define TREE_ELEMENT_VISITS 0x03
>> +#define TREE_ELEMENT_TITLE 0x04
>> +#define TREE_ELEMENT_THUMBNAIL 0x05
>
> Do these need to be public?
tree_url_node is a common factor of the usage of the tree, so
everything could be hidden in the tree-user files if each user would
be implementing it on its own. I have no idea how these could be
hidden without being redefined in each file seperately.
>> +#define NODE_INSTEP 20
>> +#define TREE_TEXT_HEIGHT 20
>> +#define FURNITURE_COLOUR 0x888888
>> +#define ICON_SIZE 16
>> +
>> +struct node;
>> +struct tree;
>> +
>> +struct node_element_box {
>> + int x; /* X offset from origin */
>> + int y; /* Y offset from origin */
>> + int width; /* Element width */
>> + int height; /* Element height */
>> +};
>> +
>> +struct node_element {
>> + struct node *parent; /* Parent node */
>> + node_element_type type; /* Element type */
>> + struct node_element_box box; /* Element bounding box */
>> + const char *text; /* Text for the element */
>> + void *bitmap; /* Bitmap for the element */
>> + struct node_element *next; /* Next node element */
>> + unsigned int flag; /* Client specified flag for data
>> + being represented */
>
> Hm. Do these values have to be globally unique?
It is up to the user what value he will assign here. In the current
use cases the values are node-unique, but still this value is needed
to distinguish the elements. Could you precise what you particularly
mean here?
