Re: Seeking your opinions on proposed patch to File::Util

[BrowserUk]

First pass opinion: When you have both --file-pattern and --dir-pattern, --pattern is redundant.

So, make --pattern & ( --file-pattern &| --dir-pattern ) mutually exclusive.

Existing code using --pattern continues to work as is.

New code can choose to stick with the old -- subject to the caveat of the known bug -- or switch to using the new mechanism.

Existing code continues to work; new code can choose to works better.

Potentially; pre-deprecate --pattern. That is, warn that it will(may) go away at some point in the future.

[martell]

Hi,

First thanks for your excellent work. I'm using it regularly.

I personally encountered the 'bug' and I worked around it by performing a list without a pattern and then grepping the whole list myself. Will you break my code with changing the behaviour of --pattern? I think not, but I have some testing to do.

Some other thoughts about the above proposal: I see a small issue with the above proposition of --file-pattern and --dir-pattern.

In list dir, you already have the switches --dirs-only and --files-only. This makes the above switches a bit confusing. I suppose the combination of the switches --files-only --dir-pattern will give me all files in the matching directories. But do I get also the files of the root dir where the search was started? Because this directory isn't matched against the --dir-pattern. And I honestly don't known what the default expectation will be.

So, either document the switches really well and watch out for unexpected behaviour like this borderline case above, or keep it simple. And I personally like simple best. So for me, you can change the --pattern behaviour and even leave the out --file-pattern && --dir-pattern. Those switches aren't crucial. Keep focussing on cross OS functionality of your toolbox, because that's the reason why I'm using your module. And if really needed, it is a simple exercise to perform the filtering ourselves.

My humble 2 cents

Martell

[Tommy]

Having evaluated all the options on the table now for several hours, I am leaning toward the more 'simple' approach you mentioned. My rationale is that I don't want to complicate things, and martel I believe you are right that the proposed new switches would make things confusing given the already existent --dirs-only and --files-only switches. With any extra switches, it's anyone's guess what the expected behavior would be from end users.

So for now I am going to plan on throwing a warning out when list_dir() is used with the --pattern switch, for at least one release (ver 3.29), expressing to the user what new behavior to expect with the following release (ver 3.30+). I don't see too much harm in this, given that if users out there are already grepping through the results returned from list_dir() against a pattern they already provided to the --pattern switch, it's not going to upset the output they are currently getting. The end-result output would be the same.

If I did go with the other option (to add more switches), then browserUK's suggestion would be the correct approach. For the present however, I'm not leaning in that direction so much as I once was, given that I feel more confident I can start enforcing the originally intended behavior without disrupting the downstream results in current user code (in what I believe will be the majority of cases).

Thoughts?

[martell]

Hi

I think throwing a warning out for something that will change in a future release isn't good practise. I personally debug until all errors and warnings are gone. In some coding environments the code repository even tests this in an automated way before accepting a commit. Your warning will disrupt this. The closest thing I've seen before is a warning that you are using an experimental feature. And that is a different situation then you are facing.

If you are really worried about code disruption, then you should not change the behaviour of the switch --pattern and use a new switch e.g. --fileter-pattern. Put the new behaviour in the new switch and throw a warning on the old --pattern switch that it is depreciated. This is the only good way to change fundamentally a behaviour with the least possible disruption for coders. Old is old and new is new. Simpler is not possible.

If it is crucial for you to change the switch --pattern, then I think it is better to change the behaviour according following plan:

Step 1: Put up a big warning message in the documentation of the switch --pattern that the behaviour will change in the next release. Put the new behaviour already in on a different switch e.g. --filter-pattern so coders can already code according the new behaviour.

Step 2: The release after, you make the switch --pattern an alias of --filter-pattern. On top of that put the old behaviour in a new switch, e.g. --pattern-no-recursive and throw a warning on the use of this switch saying that it will be deprecated on future release x. In this way people can keep the old behaviour with a simple change in their code while the warning will remind them this must be changed.

Step 3: The release after, you remove the old behaviour and then, according taste, you could depreciate the switch --filter-pattern in future release by putting first a warning and then removing the switch.

You probably now feel why I think it is better to use a new switch if you are really serious about code disruption...

And at last, but maybe the biggest advise: whatever you do, put the changes clearly in your documentation of the switches!

Again, this is only my humble 2 cents.

Martell

[Tommy]

Love these opinions! Keep 'em coming!