Re: [PATCH] historyentry work

[Vincent Legoll <vincent legoll gmail com>]

On Sun, Mar 8, 2009 at 9:15 AM, Kai Willadsen <kai willadsen gmail com> wrote:
> 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.

OK, I just resent the behavioral changes I wanted, and left the API
consistency part of it, which is orthogonal, and which I have trouble
understanding anyways: the gtk_entry and gnome_entry magic
make me stare at strange backtraces for a moment before dropping
the subject into a "to investigate" TODO list entry.

> 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.

You're right this was ugly, I implemented the behavior I wanted
without touching HistoryEntry code.

>> - 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?

The automatic activation upon selection does not prevent you from
manually editing the filename, you will be able to do so after the
activate signal has been sent (and a lot of things happened because
of that activation)

But I still prefer that to be forced to click on the text entry and then
press the enter key...

>> - 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.

Doing the saving in set_filename() allows that kind of checking, I've
added an os.path.exists() test before saving.

> 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.

I don't undestand what change is needed to NewDocDialog.on_response()
it looks fine as-is. What did I miss ?

-- 
Vincent Legoll