conig / revise

R package for writing revise and resubmits
Other
3 stars 2 forks source link

More changes... #24

Closed cjvanlissa closed 2 years ago

cjvanlissa commented 2 years ago

Most notably, this introduces a new (parallel) interface whereby the manuscript is loaded into an invisible environment variable.

conig commented 2 years ago

These are some cool changes, I paricularly appreciate that ability to attach a manuscript into the environment. I will absolutely be using that feature in all future letters.

I think we should set the argument of to_envir to FALSE by default as it's unusual behaviour and some users will be put off by the warning messages if they read the same manuscript twice.

RE: errors vs warnings. I think spelling errors in tag invokes should by default result in errors. In a long letter it could be very easy to miss the fact that a tag has been spelt incorrectly. Accordingly some users will accidentally submit letters with empty quote boxes which would cause embarrasment.

I think if we want this functionality it should be triggered by an argument + options call.

E.g.,

options(revise_errors = FALSE)

get_revision(..., revise_errors = getOption("revise_errors"))

That way if users really just want their letter to knit they can change the default behaviour in one line of code, but otherwise they will be forced to not make mistakes.

cjvanlissa commented 2 years ago

I think we should set the argument of to_envir to FALSE by default as it's unusual behaviour and some users will be put off by the warning messages if they read the same manuscript twice.

I'm not sure I agree, I think it's a very elegant interface.. I could probably magick away warning messages in most cases, e.g., by checking if a particular manuscript has already been loaded. Making it possible to load multiple manuscripts into .revise_manuscript will also make warning messages disappear. Let me work on this a bit more, I think it can be a good default behavior.

RE: errors vs warnings. I think spelling errors in tag invokes should by default result in errors. In a long letter it could be very easy to miss the fact that a tag has been spelt incorrectly. Accordingly some users will accidentally submit letters with empty quote boxes which would cause embarrasment.

That's a good point. The reason I wanted this change is because I had a problem with a tag not being found, and it stopped me from being able to work on any other aspect of my paper for a full day. That was frustrating..

I think if we want this functionality it should be triggered by an argument + options call.

That's a very good solution. Are you willing to implement that? I'll work on polishing the loading-to-environment.

conig commented 2 years ago

Thanks have added options call, you've convinced me on the default behaviour for to_envir.

conig commented 2 years ago

Apologies for hasty merge, was trying to edit but managed to merge from commandline—haven't edited someone else's PR before.

It's my first day.