Re: [PATCH 05/10] common: add _filter_trailing_whitespace

[Anand Jain <anand.jain@oracle.com>]

>>> Also, since this is so specific to the raid stripe tree, I'd rather
>>> have this filter included in the raid stripe tree filter introduced in
>>> patch 2, _filter_stripe_tree(). That would make the tests shorter and
>>> cleaner by avoiding piping yet over another filter that is used only
>>> for the raid stripe tree dump...
>>
>>    I kept this as a separate function so that it can be used elsewhere
>>    when needed. Doesn't that make sense?
> 
> Not so much if there's only one use case for it... specially if it's
> such a trivial filter...
> 

  Hmm. Yes. Also, a completely avoidable coding nitpick in btrfs-progs.

> Even if we had multiple cases, doing this pattern in the tests:
> 
> $BTRFS_UTIL_PROG inspect-internal dump-tree (... ) |
> _filter_trailing_whitespace | _filter_btrfs_version |
> _filter_stripe_tree
> 
> Is ugly and verbose. The filtering could be done in
> _filter_stripe_tree() by calling "_filter_triling_whitespace" there...
> And mentioning that, we could also call _filter_btrfs_version there,
> since it's always wanted and to make tests shorter and easier to read.
> 

  Yeah.

> So in the end it would only be
> 
> $BTRFS_UTIL_PROG inspect-internal dump-tree (... ) | _filter_stripe_tree
> 
> With _filter_stripe_tree() as:
> 
> _filter_stripe_tree()
> {
>      _filter_trailing_whitespace | _filter_btrfs_version | sed -E -e (....)
> }
> 
> Or:
> 
> _filter_stripe_tree()
> {
>      _filter_btrfs_version | sed -E -e "s/\s+$//" -e (...)
> }
> 
> A lot more clean.
> 

  I'm fine with either way. Since there is a choice here, I will keep 
the former.

  I'll send a reroll.

Thanks, Anand