tomerfiliba / plumbum

Plumbum: Shell Combinators
https://plumbum.readthedocs.io
MIT License
2.8k stars 182 forks source link

Allowed to request processing of all possible values in Set #547

Closed KOLANICH closed 1 year ago

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 83.638% when pulling d6c005a8cf2a3020f82032d093c0829228878ea9 on KOLANICH-libs:all_set into 0c574a99ef0a2419c2490db86bc8180e249c91a6 on tomerfiliba:master.

henryiii commented 3 years ago

Needs tests, Python 2 supers (for now). Also * seems problematic (shells are likely to expand it, requiring '*' to pass it in), but that's probably okay. Does this customize the help output at all? Also, why is CSV on by default? Seems like an odd departure from Set. Maybe AllCSV should be an instance of AllSet? In fact, why not use the same method used by CSV to make these, and build it in as an option to the main Set (initially empty). Then AllSet = Set(all={"all", "*"}) and it's also easy to customize by a user.

KOLANICH commented 3 years ago

Also, why is CSV on by default? Seems like an odd departure from Set.

By definition all means all the values within a set. This implies that multiple values are expected to be passed into a single argument. But just Set with csv=False accepts only a single value. Do I understand right how an ordinary Set is meant to work?

In fact, why not use the same method used by CSV to make these, and build it in as an option to the main Set (initially empty). Then AllSet = Set(all={"all", "*"}) and it's also easy to customize by a user.

I have considered that, but it can either break existing apps (if we ship all prepopulated by default) or can cause "a zoo" (if we don't, then each dev would invent an own value for all, which would be a usability issue). Both cases are undesireable. So it is proposed for the ones needing multiple values in a set explicitly opt-in into AllSet, if they need its functionality (if they allow multiple values in it, then they almost always need it).

KOLANICH commented 3 years ago

Integrated the functionality into Set, added tests and did a bit of refactoring