Add full chmod support

[timriley]

This was already supported inside the memory file system adapter. Update the main file system adapter to support this, and expose a public method.

[jodosha]

@timriley Please have a look at my inline comment about the exception class. 🙂

[jodosha]

Suggested change

	raise ArgumentError, "file mode should be an integer" unless mode.is_a?(Integer)
	raise Dry::Files::Error, "file mode should be an integer" unless mode.is_a?(Integer)

@timriley From a user perspective, when I'm using dry-files, I may want to catch all the possible errors that the lib may raise. See the following example:

begin
  # some dry-files operation
rescue Dry::Files::Error
  # handle the error
end

It starts to become cumbersome for users if we raise errors that aren't Dry::Files::Error subclasses.

[timriley]

@jodosha I agree with you here, but do you think this rule should apply for things like argument errors?

I definitely agree that having a base error class makes sense for "something has gone wrong while executing the file operation".

But "you made a programmer mistake and passed an incorrect argument" feels like a different category of error, and I would not want that to be caught by that rescue Dry::Files::Error example above, because I'd want to see the uncaught exception and use the feedback from that error to correct my mistake. (In this case, I could imagine someone passing "+x" and then learning from the exception that we support the integer mode only).

What do you think?

[jodosha]

@timriley

I would not want that to be caught by that

But how can a user know about all the possible combinations?
The only safer way for them would be to catch StandardError, which is too broad.

Remember that we're talking about handling errors, not suppressing them.
For simplicity's sake, we should stick to the rule of one superclass for raised errors.

We can also introduce a specific subclass for this case.

[timriley]

@jodosha as the primary author of this gem, I'll defer to your judgement here. I'll update this to return a Dry::Files::Error and the merge this PR.

But before I do that, let me add one more thought about what I was trying to explain earlier 😜

---

The way I see this ArgumentError here is that it's equivalent to e.g. the user forgetting even to provide a second argument in the first place. It's just that we're checking for the nature of the second argument in addition to its presence.

So a user forgetting a second argument is still an ArgumentError, and by the principle of "dry-files method errors should all be rescuable by a single error type", shouldn't we rescue for ArgumentErrors raised by Ruby itself and turn them into Dry::Files::Error? No, of course we shouldn't, because that would be silly: it goes against the grain of Ruby and would be unpredictable for the user.

In the case of an ArgumentError due to a missing argument, it's a signal that the user isn't using the API right, and they want to know about it so they can correct it, and then let any subsequent Dry::Files::Error exceptions signal that something external has gone wrong even after their correct calls to the methods.

I see this custom ArgumentError as another such case, and something the user wants to know about (through an uncaught exception) as early as possible, so they can correct their usage of the API and then get onto seeing the real meaningful errors that may come afterwards.

---

Anyway, if for any reason this changes your thinking, we can always adjust the error again afterwards. But in the spirit of just getting on with things, this is the last I'll say of it 😄

Thanks, as always, for listening ❤️

[timriley]

@jodosha This just needs a re-review!

[jodosha]

@timriley LGTM 👍