Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@

5. Non-equi joins combining an equality condition with two inequality conditions on the same column (e.g., `on = .(id == id, val >= lo, val <= hi)`) no longer error, [#7641](https://github.com/Rdatatable/data.table/issues/7641). The internal `chmatchdup` remapping of duplicate `rightcols` was overwriting the original column indices, causing downstream code to reference non-existent columns. Thanks @tarun-t for the report and fix, and @aitap for the diagnosis.

6. `fread("clipboard")` now works on Linux (via `xclip`, `xsel`, or `wl-paste`), macOS (via `pbpaste`), and Windows, [#1292](https://github.com/Rdatatable/data.table/issues/1292). Thanks @mbacou for the report, @ben-schwen for the suggestion, and @AmanKashyap0807 for the fix.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls elaborate more what this means, e.g. that fread supports now reading from the clipboard via fread("clipboard")


### Notes

1. {data.table} now depends on R 3.5.0 (2018).
Expand Down
30 changes: 30 additions & 0 deletions R/fread.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,36 @@ yaml=FALSE, tmpdir=tempdir(), tz="UTC")
# input is data itself containing at least one \n or \r
} else if (startsWith(input, " ")) {
stopf("input= contains no \\n or \\r, but starts with a space. Please remove the leading space, or use text=, file= or cmd=")
} else if (grepl("^clipboard(-[0-9]+)?$", tolower(input))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use grepl here and not simply only read from clipboard when the input is clipboard?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used regex to consider both 'clipboard' and 'clipboard-128' because original issue #1292 shows the user trying fread('clipboard-128').
However your point is valid, since readClipboard() takes no argument and only read clipboard regardless of any suffix so we can use simple string matching.
but the user who are used to base R's read.delim('clipboard-128') syntax would have to use 'clipboard'. let me know if u want the change I will do it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows, R's own clipboard connections can be opened with the description clipboard-<buffer size> in order to set the size limit for writing. It's not needed to read the clipboard. On Unix, X11_primary, X11_secondary, X11_clipboard are also supported. We don't have to support anything except clipboard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes as mentioned by Ivan, using a buffer size after clipboard- is a winwodws idiom, which we do not have to support.

I would go with identical(input, "clipboard") or input == "clipboard"

os_type = .Platform$OS.type
if (identical(os_type, "windows")) {
clip = tryCatch(utils::readClipboard(),
error = function(e) stopf("Reading clipboard failed on Windows: %s", conditionMessage(e))
)
} else if (identical(os_type, "unix")) {
sysname = Sys.info()[["sysname"]]
clip_cmd = if (identical(sysname, "Darwin")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wont this be uninitialized on Solaris and friends?

"pbpaste"
} else if (identical(sysname, "Linux")) {
if (nzchar(Sys.which("wl-paste"))) "wl-paste --no-newline"
else if (nzchar(Sys.which("xclip"))) "xclip -o -selection clipboard"
else if (nzchar(Sys.which("xsel"))) "xsel --clipboard --output"
else stopf("Clipboard reading on Linux requires 'xclip', 'xsel', or 'wl-paste' to be installed and on PATH.")
}
clip = tryCatch(system(clip_cmd, intern = TRUE),
error = function(e) stopf("Reading clipboard failed: %s", conditionMessage(e))
)
status = attr(clip, "status")
if (!is.null(status) && status != 0L) {
stopf("Reading clipboard failed (exit %d). Ensure '%s' is working.", status, clip_cmd)
}
} else {
warning("Clipboard reading is not supported on this platform.", call. = FALSE)
}
if (!length(clip) || !any(nzchar(trimws(clip)))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on non windows and non-unix this will error because of trimws(clip) but clip was never assigned

stopf("Clipboard is empty.")
}
input = paste(clip, collapse = "\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
input = paste(clip, collapse = "\n")
writeLines(clip, tmpFile <- tempfile(tmpdir=tmpdir))
file = tmpFile
on.exit(unlink(tmpFile), add=TRUE)

Using paste might allocate a large string before dumping it to disk

} else if (length(grep(' ', input, fixed=TRUE)) && !file.exists(gsub("^file://", "", input))) { # file name or path containing spaces is not a command. file.exists() doesn't understand file:// (#7550)
cmd = input
if (input_has_vars && getOption("datatable.fread.input.cmd.message", TRUE)) {
Expand Down
45 changes: 45 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -21520,3 +21520,48 @@ test(2365.1, melt(df_melt, id.vars=1:2), melt(dt_melt, id.vars=1:2))
df_dcast = data.frame(a = c("x", "y"), b = 1:2, v = 3:4)
dt_dcast = data.table(a = c("x", "y"), b = 1:2, v = 3:4)
test(2365.2, dcast(df_dcast, a ~ b, value.var = "v"), dcast(dt_dcast, a ~ b, value.var = "v"))

# Test fread clipboard input on Windows (issue #1292)
if (.Platform$OS.type == "windows") local({
temp = c("a\tb", "1\t2")
utils::writeClipboard(temp)
on.exit(utils::writeClipboard(""), add = TRUE)
test(2366, fread("clipboard-128"), data.table(a = 1L, b = 2L))
})

# Test fread clipboard input on macOS (issue #1292)
if (.Platform$OS.type == "unix" && identical(Sys.info()[["sysname"]], "Darwin")) local({
if (nzchar(Sys.which("pbcopy"))) {
con = pipe("pbcopy", "w")
writeLines(c("a\tb", "1\t2"), con)
close(con)
on.exit({
cl = pipe("pbcopy", "w")
writeLines("", cl)
close(cl)
}, add=TRUE)
test(2366.1, fread("clipboard"), data.table(a=1L, b=2L))
}
})

# Test fread clipboard input on Linux via xclip/xsel/wl-paste (issue #1292)
if (.Platform$OS.type == "unix" && identical(Sys.info()[["sysname"]], "Linux")) local({
has_wl = nzchar(Sys.which("wl-copy")) && nzchar(Sys.which("wl-paste"))
has_xclip = nzchar(Sys.which("xclip"))
has_xsel = nzchar(Sys.which("xsel"))
writer = if (has_wl) "wl-copy"
else if (has_xclip) "xclip -i -selection clipboard"
else if (has_xsel) "xsel --clipboard --input"
else ""
if (nzchar(writer)) {
con = pipe(writer, "w")
writeLines(c("a\tb", "1\t2"), con)
close(con)
on.exit({
cl = pipe(writer, "w")
writeLines("", cl)
close(cl)
}, add=TRUE)
test(2366.2, fread("clipboard"), data.table(a=1L, b=2L))
}
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing newline at end of test file

Loading