call stop instead of quit

[krlmlr]

• add test to check that --help throws error

Calling quit() is not testable and may be too harsh and even conflict with CRAN policy:

Compiled code should never terminate the R process within which it is running. Thus C/C++ calls to assert/abort/exit, Fortran calls to STOP and so on must be avoided. Nor may R code call q().

OTOH, stop() forcibly prints an error message which I have been unable to mute.

Fixes #4 (=includes it).

[trevorld]

Overall, looks great. Comments will be forthcoming on things that could be
changed (like why we should keep quit). Just got married and I have a huge
work project due the 5th so I might be slow with all my comments.

I usually ask contributors give me an Unlimited license to their
contributions so I can change the overall package license in the future
which I don't forsee doing in the near future but maybe I'll want to go BSD
or GPL-3 or something.

Thanks,

Trevor

On Sun, Nov 24, 2013 at 1:15 AM, Kirill Müller [email protected]:

• add test to check that --help throws error

Calling quit() is not testable and may be too harsh and even conflict
with CRAN policy:

Compiled code should never terminate the R process within which it is
running. Thus C/C++ calls to assert/abort/exit, Fortran calls to STOP and
so on must be avoided. Nor may R code call q().

OTOH, stop() forcibly prints an error message which I have been unable to
mute.

Fixes #4 #4 (=includes it).

You can merge this Pull Request by running

git pull https://github.com/krlmlr/optparse quit

Or view, comment on, or merge it at:

#5
Commit Summary

• enable Travis CI integration via craigcitro/r-travis
• add RStudio project
• call stop instead of quit
• check that --help throws error

File Changes

• M .Rbuildignorehttps://github.com/call stop instead of quit #5/files#diff-0(3)
• M .gitignorehttps://github.com/call stop instead of quit #5/files#diff-1(2)
• A .travis.ymlhttps://github.com/call stop instead of quit #5/files#diff-2(24)
• M R/optparse.Rhttps://github.com/call stop instead of quit #5/files#diff-3(2)
• M inst/tests/test-optparse.Rhttps://github.com/call stop instead of quit #5/files#diff-4(1)
• A optparse.Rprojhttps://github.com/call stop instead of quit #5/files#diff-5(16)

Patch Links:

• https://github.com/trevorld/optparse/pull/5.patch
• https://github.com/trevorld/optparse/pull/5.diff

[krlmlr]

Congratulations, and good luck!

Thanks. You could still apply #4 first, #5 (or a modified version) can be applied on top.

One more reason not to call quit() is that it will presumably end a session in RStudio/RGui/R interactive/... . stop() works just as well for all practical purposes, short of forcibly emitting a message. Perhaps we can work around this?

Unlimited license to use my contributions under any open-source license is granted herewith.

[trevorld]

Calling quit does technically break CRAN policy but whenever I submit a new version of optparse or argparse to CRAN I usually draw attention to it with something like "NB. As previously disclosed and documented this package, if the parser sees an help option, will call quit() after printing a usage message.". So it looks like, at least for the moment (although CRAN is known for changing directions), for this very narrow use case CRAN won't make me change this especially if the examples and tests don't directly call quit. The package isn't intended for interactive use where the use of "quit" is most surprising and malicious.

The new forcibly emitted error message would surprise and bug current package users.

Theoretically you could indirectly test whether it was quitting properly by writing a utility Rscript that cats info at the end after it calls parse_args. You could then use system function and intern the output to see if it was working correctly by checking whether the help message was being printed out and the extra info was not being catted out. The package vignette effectively runs this test in a non-automated fashion.

[krlmlr]

But what about interactive use in RStudio et al.? Although not designed for interactive use, I have "hybrid" scripts that should work in both RStudio and standalone. There should be a way not to call quit(), and calling quit() shouldn't even be the default IMHO.

Users who rely on quitting can use

tryCatch(..., error=function(e) quit())

We can provide a wrapper or an optional parameter in parse_args.

Actually, there would be ways to mock quit() to allow for automated testing. This is R, after all ;-) Still, not calling it forcibly would be better, I think.

[trevorld]

What are your specific interactive via RStudio use cases? Already if you want to interactively see help in R without quitting you can use print_help(parser) although the standard way to interactively see help for an Rscript would be Rscript my-script-name.r --help on a terminal. If you want to interactively parse arguments without quitting you can use parse_args(parser, print_help_and_exit = FALSE) or simply leave out any '-h' or '--help' arguments out (for example if you wanted to populate some options with your defaults when debugging a script interactively). i.e. I have Rscripts that look something like this:

if (interactive()) {  # what I get if I source my Rscript from interactive R
    opts_and_args <- parse_args(parser, vector_of_debugging_args)
} else { # what I get if I call from command-line
    opts_and_args <- parser_args(parser) 
}

The primary purpose of the package is to be used non-interactively in a Rscript. Users have been writing scripts with optparse for over 4 years and I don't want to be breaking any backwards compatibility -- additionally for any new users familiar with the Python optparse package this package is inspired by it would break their expectations if it did not call quit which is pretty standard for higher level option parsers to do after printing help in response to seeing a help option. A tryCatch statement would require an extra line of code which most users would now need to add to their scripts.

[trevorld]

Github won't let me re-open this thread about stop versus quit when seeing --help on the command-line even though I reversed your specific commits addressing this issue when I manually merged #7 ...

[krlmlr]

It's just that I really need to be careful not to pass --help or -h to the option parser. Also, this would avoid having to remember the print_help routine.

Why don't we call stop() if interactive(), and quit() otherwise?

[krlmlr]

See also the help to stop():

The default behaviour (the NULL error-handler) in interactive use is to return to the top level prompt or the top level browser, and in non-interactive use to (effectively) call q("no", status = 1, runLast = FALSE)

[trevorld]

Seems reasonable.

Made commit so that development version now only calls quit if
!interactive() (and print_help_and_exit) otherwise (if
print_help_and_exit) calls stop.

On Sun, Dec 15, 2013 at 4:30 PM, Kirill Müller [email protected]:

It's just that I really need to be careful not to pass --help or -h to
the option parser. Also, this would avoid having to remember the
print_help routine.

Why dont we callstop()if interactive(),
andquit()` otherwise?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-30626253
.