> > Any reason for moving "case LOFS_BLOB" (and "case LOFS_BEGIN_TREE" > > below) after LOFS_END_TREE? > > I put LOFS_BLOB and after LOFS_END_TREE since that is the order in all > the other filter logic functions. I put LOFS_BEGIN_TREE at the end > (which is different from the other filter logic functions) because it's > usually better to put simpler things before longer or more complex > things. LOFS_BEGIN_TREE is much more complex and if it were not the last > switch section, it would tend to hide the sections that come after it. > > FWIW, I consider this the coding corollary of the end-weight problem in > linguistics - see https://www.thoughtco.com/end-weight-grammar-1690594 - > this is not my original idea, but something from the book Perl Best > Practices, although that book only mentioned it in the context of > ordering clauses in single statements rather than ordering entire blocks. OK - my thinking was that we should minimize the diff, but this reasoning makes sense to me. > > Here, filter_trees_update_omits() is > > only ever used to remove a blob from the omits set, since once this blob > > is encountered with include_it == true, it is marked as LOFR_MARK_SEEN > > and will not be traversed again. > It is possible that include_it can be false and then in a later > invocation it can be true. In that case, the blob will be added to the > set and then removed from it. Ah...yes, you're right. > For your reference, here is an interdiff for this particular patch after > applying your comments: The interdiff looks good, thanks. All my issues are resolved.
