Re: [PATCH] CachedVC: Add ability to update file status when its modified

[Kai <kai willadsen gmail com>]

On 11 January 2011 14:00, Peter Tyser <ptyser gmail com> wrote:
<snip>
>> So this looks like: if we don't get any results, we assume that the
>> path is normal, but if we get results, anything not mentioned gets the
>> NONE state instead. The second part of this sounds right to me, but
>> I'm wondering why the first part is needed; it wasn't part of the
>> previous logic, so what's different?
>
> The first part of the conditional is needed now that this function is
> used to update the tree state, specifically when a file changes from
> modified to non-modified.  Previously it always was called to create a
> fresh tree so this wasn't necessary.
>
> For example, if 'AUTHORS' is modified when the tree state is first
> populated, then its modifications are manually reverted so that it is
> no longer modified, we need to update 'AUTHORS' state to STATE_NORMAL
> (STATE_NONE would also work, but I like STATE_NORMAL personally).

I agree.

As an aside, I noticed that the treeview isn't actually refiltered
here, so after the file reverting to STATE_NORMAL, it still appears in
the treeview even when normal files should be filtered out. I thought
about 'fixing' this, and then decided that the current behaviour was
actually better. I think that when you're reviewing changes, it makes
it obvious that there are a set of files that were different, and
which you're now done with.

> Without that first check the get_path_state() wouldn't know that
> 'AUTHORS' needs to be updated so it'd remain in its "modified" state.
> There are some other ways to accomplish the same thing, but the logic
> is generally pretty similar.  I'll beef up the comment in the next rev
> and feel free to chime in if someone has a cleaner way.

Thanks for the explanation. The updated patch looks great, so I've
just pushed it.

cheers,
Kai