feat!: allow overwriting rextendr templates by use_extendr

[eitsupi]

Resolve #275

This PR allows the use_extendr function to overwrite existing templates.
If it is not an interactive session, skip without overwriting.

[eitsupi]

@JosiahParry Could you take a look at this?

[Ilia-Kosenkov]

Please also update NEWS.md since it is a significant change of the behavior.

[eitsupi]

Please also update NEWS.md since it is a significant change of the behavior.

I think I have already done that.

[Ilia-Kosenkov]

I think I have already done that.

Sorry, sometimes I am just blind to the things I see in front of me :)

[JosiahParry]

I can give this a review tomorrow!

[JosiahParry]

This is the behavior I received with your new branch! I quite like it. At first I hesitated with not having an overwrite argument, but it would be really terrible to not realize your lib.rs got overwritten as well.

I would, personally, like for there to be a use_makevars() function or similar so that those files can be updated. I will make a new issue for it.

[eitsupi]

Thanks for reviewing this.

Yeah, overwriting lib.rs and Cargo.toml is not ideal.
I considered skipping these by default, but I think we should keep the option given the package renaming.

I would, personally, like for there to be a use_makevars() function or similar so that those files can be updated. I will make a new issue for it.

Creating a separate function may be excessive; how about adding an option to use_extendr so that the behavior can be changed to use only those files?

[JosiahParry]

...how about adding an option to use_extendr so that the behavior can be changed to use only those files?

My hesitancy here is that once you start adding too many option to a function it becomes a bit like a magical cauldron that needs special incantations to work how you want it--i.e. the purpose becomes less straight forwrd.

Alternatively, rextendr:::use_rextendr_template() could be made part of the public API so that template files are easier to use.

[JosiahParry]

Once the typo in the doc is fixed and regenerated in the man page I think its good to go!

[JosiahParry]

I "approve" this PR! Looks quite good.

[JosiahParry]

I'm not able to actually approve so I'm putting up the bat signal @Ilia-Kosenkov

[CGMossa]

I'll review it then; @JosiahParry you should be able to approve.. We'll investigate that too...

[eitsupi]

Approval itself can be done by anyone (There is a repository option to disallow it), but the green check is only given to approvals by those who have the permission to merge, I think.

[CGMossa]

This is a lot. LGTM. I'll wait for @Ilia-Kosenkov to give it a look. But @JosiahParry is a contributor as well, so this is approved.

[JosiahParry]

Upon further review, why is the overwrite argument not provided? Rather it is inferred by interactive(). Could we, instead, have overwrite = NULL as the default argument so that it asks you by default. Then users can provide the option if they so desire

[Ilia-Kosenkov]

If there are still comments, I will merge as soon as they are resolved.

[eitsupi]

Upon further review, why is the overwrite argument not provided? Rather it is inferred by interactive(). Could we, instead, have overwrite = NULL as the default argument so that it asks you by default. Then users can provide the option if they so desire

Indeed, it is annoying to be asked several times if we want to overwrite. I have updated it.

[JosiahParry]

@Ilia-Kosenkov all resolved

...