On Thu, Sep 4, 2008 at 8:41 PM, Petr Rockai <[EMAIL PROTECTED]> wrote:>
David Roundy <[EMAIL PROTECTED]> writes:
>> (And I'd want to do a behavior change in smart_diff, so if you can wait on
>> that until we've had time to talk, that would be great.)
> I have noticed something in that respect, and I'd like such a change myself,
> too. If I understood correctly, and also my intuition would be to:
>
> 1) smart_diff doesn't need to know about either LookForAdds or
> Summary/NoSummary -- afterall, if the (extraneous) information is not used in
> the upper layer, I think it should stay unevaluated so no overhead incurred
> (the Slurpy principle).
>
> - It seems Summary is used to ignore file content on addfiles.

This is a simple performance hack, of course...

> - LookForAdds is used to avoid those files that are only in working.

The LookForAdds behavior, I think, should always be treated as
present, since LookForAdds is already handled at an earlier stage.
This is the main change I'm interested in seeing made, and should be
kept separate from any (other) refactor, since it's a major change in
the workings of this function.  This should simplify the behavior of
smart_diff considerably (removing one of the three ugly opts
entirely).

The existence of LookForAdds support in smart_diff dates back to
around 2002 or 2003, before co_slurp  had been written to avoid the
extreme  inefficiency of filtering  untracked files in smart_diff.
Since then, this functionality has been useless (and in some cases
buggy, e.g. in check).

> Now, I haven't looked how addfile patches actually work, but I'd guess that
> smart_diff doesn't look at pending, so any "addfile" patches in its output
> would indicate an untracked file (that would be currently excluded without
> LookForAdds). (I've quickly looked at get_unrecorded_private and what I see
> seems to support that theory.
>
> This means the upper layer has enough information to filter whatever it wants
> to (in this case, I suppose get_unrecorded is the user that wants that kind of
> handling for LooksForAdds, or potentially even higher the stack -- but in that
> case, get_unrecorded might need to mark up the patches as "automatic" and 
> "from
> pending").

One option would certainly be to push the whatsnew --summary
performance hack to a different level.  In the short term, we could
also live without it.  I'm not convinced that pushing this hack into
higher-level code is a  particularly good idea.

> 2) There is still the issue of IgnoreTimes: We can do a similar thing like 
> with
> Compression, only passing in the information whether to honour or ignore
> timestamps.
>
> However, if we go one further layer up, it might be worth to start thinking 
> how
> to concisely combine these minimalistic pieces of information into slightly
> larger blocks. On the other hand, I probably shouldn't worry about that just
> yet, since it's hard to tell how will things end up like. Maybe just tuple the
> various types, and if a certain tuple turns out to be common and cohesive,
> newtype it and add a [DarcsFlag] -> WhateverNewType function?
>
> Well, I've just went a little wild-guessing on your intentions. I'll wait with
> doing any coding till I hear from you on what you have actually intended. I've
> just been thinking that it might save time if I guess rightly (or give an
> alternative viewpoint if I haven't).

As I mention above, the first  thing to do is to (separately) remove
the unneeded and bug-inducing functionality.  That eliminates one of
the three flags.   At that point we could consider whether we want to
enshrine the whatsnew --summary hack in the type system, or move the
hack into a different function ("get_unrecorded_summary"?), or maybe
just leave things as they are.

David
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to