make mkpath method returns `$self` on succes if `wantarray` is false

[nataraj-hates-MS-for-stealing-github]

Hi!

Would you consider a patch that changes the behavior of mkpathmethod, if wantarray is 0?

I would expect it to return $self on success (if wantarray is 0), so it will be possible to following thing

path('foo/bar')->mkpath->child('my_file')->spew($text);

As far as I can get, this should not break any of existing code, as current usage of return values make sense only with wantarray !=0

If you consider this an acceptable behavior change, I would create and submit a patch for it.

[xdg]

mkpath matches the semantics of File::Path::make_path so I don't want to change the existing return value.

However, I see what you're trying to accomplish, and it reminds me of the request in #251. Is there a reason not to use touchpath, since the file will be overwritten by spew anyway?

path('foo/bar')->child('my_file')->touchpath->spew($text);

[nataraj-hates-MS-for-stealing-github]

path('foo/bar')->child('my_file')->touchpath->spew($text);

First, thank you for letting me know about this solution, I will use it in my current code.

Nevertheless I should say that for me this solution is totally not obvious, if you are not Path::Tiny expert. touchpath concept itself is no very obvious too. I tried to search the net, it seems that it is Path::Tiny's invention. The least thing I would suggest here is to mention touchpath in mkpath documentation. For example this would lead me to the right direction and fit my current needs as a Path::Tiny' user.

But as a developer, I will still try to oppose. Current mkpath behavior is not Path::Tiny way, as I expect it. Most of the methods return another Path::Tiny object (when they are not supposed to return something else on purpose). As a Path::Tiny user I really do not care what original perl module is behind each method, I would expect consistency in module behavior first, and only that I would expect then to inhere features of modules that actually implements this behavior.

Another example, I would expect this to work:

path('foo/bar')->mkpath->chmod(0770);

I know, that I can theoretically do

path('foo/bar')->mkpath(mode => 0770)

but this require me to know something about File::Path and it's ways. And the whole idea of Path::Tiny is that I can forget about File::Path and other modules and use Path::Tiny instead. And example above breaks this concept.

So I will continue saying that returning Path::Tiny object when mkpath is called in scalar context is a very good idea from my point of view.

[ap]

Path::Tiny’s mkpath is not a recent interface, and its current return value in scalar context is not useless to the point that nobody could conceivably be using it… so changing mkpath is out.

So then the question become whether there is a sufficiently common use case that it’s worth bloating the interface with another (and partially redundant) method.

Nevertheless I should say that for me this solution is totally not obvious, if you are not Path::Tiny expert. touchpath concept itself is no very obvious too

You can’t be faulted for that. I always have to remind myself despite its name it doesn’t touch every directory along the path but is just mkpath + touch + return $self. (I also have to admit I can see myself naming this method the same unfortunate way if I were writing it; it feels like it would seem like the obvious concise choice of name for the method.)

The least thing I would suggest here is to mention touchpath in mkpath documentation.

@xdg: This seems downright necessary.

[xdg]

As a Path::Tiny user I really do not care what original perl module is behind each method

Heh. Victim of my own success, I guess. :grin:

I agree that the docs should be improved. I'm going to continue to ponder whether there should be chainable alternative to mkpath. I see the point, but it's not "Tiny" to keep adding to the API.

[ap]

I’ve been thinking about it since the last comment and am leaning toward adding this on balance.

it's not "Tiny" to keep adding to the API.

This is my main misgiving too. But I’ve come around to the view because I now think that ultimately touchpath was a design too clever by half. And I’ll get to why after I explain what I think should be added. (And I’m not saying this to disparage the designer. I can see logic that would likely have led me to the same place, and it’s only with hindsight that I’m arguing it mistaken.)

Namely, I think there should be a chainable method which can be called to create all the directories in the path part of a Path::Tiny instance.

That is, the method should be callable both on directory instances, where it simply acts as a chainable version of mkpath – but also on file instances. Its effect on file instances should amount to $file->parent->mkpath only, chaining back to $file, but without operating on the file itself in any form. More concretely, the following lines would both do the exact same thing:

path('foo/bar/baz/')->NEWMETHOD->child('quux.txt')
path('foo/bar/baz/quux.txt')->NEWMETHOD

Why this particular design? Because it fixes what touchpath got wrong:

1. It does not combine two operations that prospective callers might not necessarily want both of.

   Many callers with file objects who don’t need the touch part of the combination may be able to ignore it, but the API nevertheless forces them to request this superfluous step, which a reader of the code may then have to wonder if it is actually necessary.

   And callers with directory objects cannot use it at all. Those have mkpath available of course, but that method is not chainable.

2. By limiting itself to operating on directories only, this new method can trivially be designed to accept options that pertain to directory creation only.

   With touchpath the combination of operations makes this a design conundrum. It is unclear which of the operations to prioritize, or else how to design a clear interface for combining options relating to both operations.

3. I am actually not entirely sure whether path('foo/bar/baz/')->NEWMETHOD should create all of foo/bar/baz/ or just foo/bar/. Both are plausible, since the method chains back to a Path::Tiny instance which points to the the tail directory in the path.

   The argument in favor of creating the entire path is that makes it easy to apply the same options to all of the directories without having to repeat this information for the tail directory. And if you want to apply different permissions/ownership to the the tail directory then you have to do that in an extra chmod (or whatever) call chained onto NEWMETHOD anyway. (Which you can, easily, because NEWMETHOD chains back to the original path.)

   OTOH, if you do want to apply different options to the tail directory, then having NEWMETHOD apply other options first before you go in and flip them back over would force doing it in a “two steps forward, one step back” sort of way, as far as observable side effects on the file system. I’m not sure whether this might not under some circumstance have security-relevant repercussions. And there is also the argument that it would be more consistent if NEWMETHOD never operates on the tail component of the path, be that a file or a directory.

   At this time, I’m leaning toward creating the tail directory overall, but I wonder whether I am just repeating one of the mistakes I would like to get away from in touchpath.

4. As for the name of this new method I was all set to suggest following precedent and calling it assert after IO::All’s similar-ish method.

   But then I looked at what IO::All::assert actually does, and while it bears some similarity in its use to what I am proposing, that method is actually a rather higher-level concept which does not immediately perform any filesystem operation at all. Instead it requests that the directories above the tail component of the path be created if necessary if and when any operation on the tail component causes that to be created.

   So now I will argue only that NEWMETHOD should be called something other than assert, to avoid suggesting closer similarity than these methods would have.

If it were possible I would remove touchpath after adding NEWMETHOD, but the best that can be done is do reduce the touchpath documentation to a stub that just says basically “same as $file->NEWMETHOD->touch but you cannot pass any options”.

[xdg]

I now think that ultimately touchpath was a design too clever by half

I'm fairly convinced, but a bit overloaded with work/life, so please don't take silence as disagreement or an attempt to warnock the issue.

[nataraj-hates-MS-for-stealing-github]

I needed weekend to read that lot of text ;-)

As a Path::Tiny user, I would really be confused if path('foo/bar/baz/')->NEWMETHOD create baz dir and path('foo/bar/baz')->NEWMETHOD create baz file.

So I would suggest, that NEWMETHOD should always create parent dir for consistency, paying no attention to railing slash

While reading your comments following idea came to my mind, NEWMETHOD can be mkparent, and can be used like this

path('foo/bar/baz/my_file')->mkparent->spew($my_text);

And if for some reason you need to create path and then chain it, I guess you can do

path('foo/bar/baz/.')->mkparent->child('my_file')->spew($my_text);

[ap]

As a Path::Tiny user, I would really be confused if path('foo/bar/baz/')->NEWMETHOD create baz dir and path('foo/bar/baz')->NEWMETHOD create baz file.

You are of course correct. I don’t know how I missed that.

While reading your comments following idea came to my mind, NEWMETHOD can be mkparent,

Yeah. If it won’t create the tail component then that name seems fairly obvious. Maybe mkparents to make it clear that it isn’t limited to just the immediate parent directory.

And if for some reason you need to create path and then chain it, I guess you can do

Indeed. This may be a bit too subtle to advertise officially… but certainly a logical ramification of the API design.

[xdg]

I'm just thinking out loud here so I don't forget my train of thought -- not committing to any course of action yet.

• I don't like the idea of a "make parent" method, because generally the API acts on the path object itself. Even touchpath is "do everything needed to make this path exist"
• I think the most natural thing is "mkpath but returns $self for chaining". Possibly this is just a matter of finding the right name. Ideas: make_path (except having the preferred thing be longer sucks), mkdir (different semantics than built-in mkdir, but is short, clearly indicates a directory-related operation, and would appear in docs next to mkpath), make_dir (avoids built-in collision), touchdir (not great because it doesn't actualy touch a directory, but alpha sorts right next to touchpath). If forced to pick one now, mkdir seems the least worst option. (Or mkdir_p for association with mkdir -p on the CLI? But that's a very Unixy affordance.)
• Another option is to allow touchpath take options for mkpath and touch: $path->touchpath( $time, \%options ). Possibly add an option dir_only so that $path->touchpath( {dir_only=>1} ) is effectively the previous idea for people who really need a chainable mkpath. This avoids growing the API, which I like, but it's much more verbose than something like $path->mkdir->chmod(0770). This is also not mutually exclusive with the previous option -- could add mkdir and let touchpath take arguments.

Another idea -- probably terrible -- would be a way to embed a different chain within a chain:

path("foo/bar/baz")->with( sub { $_->parent->mkpath } )->spew($txt)

# or

path("foo/bar")->with( sub { $_->mkpath } )->child("baz")->spew($txt)

If I'm going to expand the API, adding a general tool over a duplicate specific one could be useful. Would people want/use this for real purposes to do stuff inside a chain? E.g.

$path->with( sub { say $_ } )->spew($txt)

Is that even Tiny?

[xdg]

I decided to just call it mkdir and call out why it's different than the built-in in the documentation.

[nataraj-hates-MS-for-stealing-github]

Thanks!!