2009/3/7 Vincent Legoll <vincent legoll gmail com>:
> I was trying to get VcView.fileentry to behave more
> sanely, and created the attached patch. It seems to
> work better now, but as I really don't know what I am
> doing, I would like people to review the changes made,
> and tell me if they are {,in}sane.
Firstly, thank you for checking out the HistoryEntry stuff. These
classes are not pretty, partially because I translated them (poorly)
from C, and partially because the libgnomeui APIs that they mimic are
really old... and feel it.
This is also the source of some of the problems you found. The idea
was to closely mimic the existing behaviour, though having done that
we could also easily adapt them to behave as needed.
> What was not working:
> - vcview fileentry history was not saved
> - need to press enter after selecting an history element
>
> What's inside:
> - don't use knowledge of HistoryEntry internals in HistoryFileEntry (1)
I agree that we shouldn't be checking .child, but HistoryEntry already
has a get_entry() call that should be used in these places (see
attached patch). The reason I don't like the new methods is that we
then have get_text() and set_text() that operate on the text in an
entry, and append_text() and prepend_text() that operate on the
history-based list, and I think that looks very confusing.
I also don't agree with prepending to the history on a set_text()
call. There are lots of reasons to set the text of the entry that
shouldn't be saved.
> - send activation signal if an element from the drop down menu has been selected
I just checked out GnomeEntry, and it seems to work this way as well,
where selecting a line from the history only fills the entry with
doing anything else. So I'm not sure about this change. I can see the
benefit, but there are downsides as well (e.g., not being able to
select from the list and then edit). Other opinions?
> - move currently selected element to the top of the history upon activation
This is related to the "history not saved" comment above.
So with the current API, users of HistoryFileEntry have to explicitly
request that things be saved. The idea is that you don't always want
whatever is entered to be saved (when I mistype a path name, for
instance, it would be nice if that was *not* saved) and without some
external signal, it's not clear *when* it should be saved. I'm not
saying that this is how it should work, just how it does work.
Having said that, I think there is currently only one place where we
actually save file entry history properly, so your change to make
history automatically prepended on activation is okay by me. However,
if this change is made, then NewDocDialog.on_response() will have to
be fixed to work with the new semantics.
Kai
Attachment:
HistoryPrivate.patch
Description: Binary data