On Mon, 2009-07-06 at 18:51 +0200, Paweł Blokus wrote:
> 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.
Perhaps. As I said, I'm not sure why this exists, anyway :)
> 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.
It may be better to remove the recent URLs stuff from the core
completely. The reason it exists is because the RISC OS frontend has a
menu containing the 10 URLs most recently entered into the URL bar.
Given that we have URL completion, it may be that this feature is never
used and can be removed wholesale.
> >> 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 :)
Ok.
> >> +#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.
I guess I imagined that the URL node code would care about this stuff
and provide accessors for its clients.
> >> +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?
That clarifies things, thanks. I just wasn't entirely sure how you
envisaged this flag being used.
J.
