`exists` should check directories

loreanvictor / tmplr

Automate Code Scaffolding

MIT License

26 stars 0 forks source link

`exists` should check directories #36

Open loreanvictor opened 9 months ago

loreanvictor commented 9 months ago

exists does not check existence of directories. this can be confusing, and also is inconsistent with behaviour of remove, where if a directory is explicitly specified (without any glob patterns) it will be removed.

originally brought up in #35 by @fwextensions.

loreanvictor commented 9 months ago

some thoughts:

current behaviour is not intuitive and at least should be documented.
it sounds reasonable for exists to behave similarly to remove, i.e. when no glob pattern is provided then existence of directories is also checked.
on the other hand, while it is common use-case for remove to remove empty directories for cleanup, in most use-cases (that I can currently think of) the recipe should behave the same when some particular directory doesn't exist and where it exists but it is empty. this means, most of the time this is the correct, intended code:
```
exists: dir/**/*
```
and this is wrong:
```
exists: dir/
```
tmplr could automatically treat dir/ as dir/**/*, but that is a bit of a magical behaviour and might result in further confusion.

considering all of this, I think currently the best course of action is merely to add this behaviour to the documentation, unless use-cases for checking existence of empty directories are brought up.

loreanvictor commented 9 months ago

I added the hint to the docs. Will keep this issue open for now for further discussion on the matter.

fwextensions commented 8 months ago

the recipe should behave the same when some particular directory doesn't exist and where it exists but it is empty

That may often be true in practice, but I don't think it has anything to do with how exists: dir/ should behave. The current behavior seems like a bug, since it evaluates to false even if dir exists and contains files.

Directories exist whether or not they contain anything. If I just ask whether dir exists (because I don't care if it's empty or not), then a command called exists should tell me. If I care whether it's empty, I can check for dir/**/*.

Actually, I could imagine the hypothesis above being true for the remove command if it displayed an "are you sure?" confirmation. If the directory didn't exist or was empty, it could silently do its thing, while it would show the confirmation only for non-empty directories. But I don't think exists should presuppose anything about how it's being used, and should just return accurate information.

loreanvictor commented 8 months ago

I understand the logical correctness perspective. My point is, checking existence of directories seems like a foot-gun in most cases (you do it because it is easier to do and seems correct while it is not), unless I'm missing some use-cases.

A counter argument is perhaps that the current behaviour is not adequate in preventing this foot-gun? Perhaps looking at various use cases helps this discussion.