Skip to content

Commit

Permalink
Update sink.R
Browse files Browse the repository at this point in the history 
The default value of type is a vector but implementation assumes a single character values. This is very confusing from an interface perspective. Proposing to change the default arg to one character value instead.
  • Loading branch information
@hhamid
hhamid committed Mar 17, 2015
1 parent b886e32 commit d3cad22
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/library/base/R/sink.R
View file
Expand Up @@ -16,7 +16,7 @@
# A copy of the GNU General Public License is available at
# http://www.r-project.org/Licenses/

sink <- function(file=NULL, append = FALSE, type = c("output", "message"),
sink <- function(file=NULL, append = FALSE, type = "output",
split=FALSE)
{
type <- match.arg(type)

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Mar 17, 2015

For this to work, it's be required that the default value of type is indeed c("output", "message") as in the code -- see ?match.arg. You will find this pattern in many other functions.

This comment has been minimized.

Sign in to view

Copy link
@hhamid

hhamid via email Mar 17, 2015

Author Owner

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Mar 17, 2015

@hhamid: Unfortunately, your proposition can't work the way it should. Please, see ?match.arg:

In the one-argument form ‘match.arg(arg)’, the choices are
obtained from a default setting for the formal argument ‘arg’ of
the function from which ‘match.arg’ was called.  (Since default
argument matching will set ‘arg’ to ‘choices’, this is allowed as
an exception to the ‘length one unless ‘several.ok’ is ‘TRUE’’
rule, and returns the first element.)

The best you can do is to point out in the documentation that character vectors are not accepted.

This comment has been minimized.

Sign in to view

Copy link
@hhamid

hhamid via email Mar 17, 2015

Author Owner
Expand Down

0 comments on commit d3cad22

Please sign in to comment.