Skip to content

Simplify RStudio Addin #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 19, 2017
Merged

Simplify RStudio Addin #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
13d0663
use `rstudioapi::modifyRange()`
lorenzwalthert Sep 5, 2017
2b875d8
use rstudiapi::insertText()
lorenzwalthert Sep 18, 2017
64d9d22
Revert "use rstudiapi::insertText()"
lorenzwalthert Sep 18, 2017
67efab2
add test
lorenzwalthert Sep 18, 2017
3fac3a9
add non-working test
lorenzwalthert Sep 19, 2017
5eab449
Revert "add non-working test"
lorenzwalthert Sep 19, 2017
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
102 changes: 7 additions & 95 deletions R/addins.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#'
#' Helper function for RStudio Addin.
style_active_file <- function() {
context <- find_active_context()
context <- get_rstudio_context()
style_file(context$path, style = tidyverse_style)
}

Expand All @@ -13,103 +13,15 @@ style_active_file <- function() {
#' one thing: You can highlight also just parts of lines.
#' @importFrom rlang seq2
style_active_region <- function() {
context <- find_active_context()
if (all(context$start == context$end)) stop("No region selected")
all_text <- utf8::read_lines_enc(context$path)
styled_expr <- style_region(all_text, context)
neighbourhood <- extract_neighbourhood(all_text, context)

styled_lines <- complete_styled_expr(context, styled_expr, neighbourhood)

merged_text <- append(neighbourhood$lines_not_to_style, styled_lines, context$start[1] - 1)
utf8::write_lines_enc(merged_text, context$path)
}

#' Helper to extract and preprocess relevant attributes from
#' [rstudioapi::getActiveDocumentContext()].
#'
find_active_context <- function() {
context <- get_rstudio_context()
path <- context$path
start <- context$selection[[1]]$range$start
end <- context$selection[[1]]$range$end
if (end[2] == 1 & !all(start == end)) {
end[1] <- end[1] - 1
end[2] <- 1000000L # because of range constraint in substr()
}
list(start = start, end = end, path = path)
text <- context$selection[[1]]$text
if (all(nchar(text) == 0)) stop("No code selected")
out <- style_text(text)
rstudioapi::modifyRange(
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract a wrapper around modifyRange() and mock that wrapper?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The problem is that rstudioapi assigns the document ID dynamically, and modifyRange() needs this ID to select the document to modify, so I think we can't mock. The approach would have been something like 3fac3a9.
I suggest we just leave it without testing for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The problem is that when modifying the file with modifyRange() we need the document id of the file to modify, which we get with getActiveDocumentContext() when run interactively. However, I don't see a way to get id without running getActiveDocumentContext() and ids are assigned dynamically (which can be tested by simply opening a file and tet the active context of it, closing it and get the context again).

context$selection[[1]]$range, paste0(out, collapse = "\n"), id = context$id
)
}

get_rstudio_context <- function() {
rstudioapi::getActiveDocumentContext()
}

#' Style a region of text given context
#'
#' First extracts the relevant expression from `text` and returns after styling.
#' @param text Character vector that contains the highlighted region.
#' @param context The context from [find_active_context()].
style_region <- function(text, context) {
ind_to_style <- seq2(context$start[1], context$end[1])
lines_to_style <- text[ind_to_style]
last <- length(lines_to_style)
lines_to_style[1] <- substring(lines_to_style[1], context$start[2])
start_if_on_same_line <- ifelse(context$start[1] == context$end[1], context$start[2], 0)
lines_to_style[last] <- substring(
lines_to_style[last],
1,
context$end[2] - start_if_on_same_line
)
styled_lines <- style_text(lines_to_style)
styled_lines
}

#' Complete styled expression with unstyled fraction on start / end line
#'
#' Essentially adding the parts before the highlighted area on the line where
#' the higlighted area starts to the styled expression. Also, after the styled
#' expression, the remainder of that line that was not styled is added.
#' @param context The context from [find_active_context()].
#' @param styled_expr Character vector consisting of the styled expression.
#' @param neighbourhood Neighbourhood obtained from [extract_neighbourhood()].
complete_styled_expr <- function(context,
styled_expr,
neighbourhood) {
if (context$start[1] == context$end[1]) {
styled_expr[1] <-
paste0(neighbourhood$start, styled_expr[1], neighbourhood$end)
} else {
last <- length(styled_expr)
first_line <- paste0(neighbourhood$start, styled_expr[1])
last_line <- paste0(styled_expr[last], neighbourhood$end)
styled_expr[1] <- first_line
styled_expr[last] <- last_line
}
styled_lines <- styled_expr
styled_lines
}

#' Extract text around the highlighted text
#'
#' Extracts unselected text on highlighted lines and text on other lines
#' @param text Character vector that contains highlighted area.
#' @param context The context from [find_active_context()].
#' @return A character vector of length three.
#' * The first element corresponds to the text on the start line that was not
#' selected.
#' * The second element to the text on the end line that was not selected.
#' * The third element are the lines that are not overlapping with the
#' highlighted text.
extract_neighbourhood <- function(text, context) {
ind_to_style <- seq2(context$start[1], context$end[1])
lines_to_style <- text[ind_to_style]
lines_not_to_style <- text[setdiff(seq_along(text), ind_to_style)]
fract_of_line_start <- substring(lines_to_style[1], 1, context$start[2] - 1)
last <- length(lines_to_style)
fract_of_line_end <- substring(lines_to_style[last], context$end[2])
list(
start = fract_of_line_start,
end = fract_of_line_end,
lines_not_to_style = lines_not_to_style
)
}
20 changes: 0 additions & 20 deletions man/complete_styled_expr.Rd
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions man/extract_neighbourhood.Rd
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions man/find_active_context.Rd
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions man/style_region.Rd
View file
Open in desktop

This file was deleted.

39 changes: 2 additions & 37 deletions tests/testthat/test-public_api.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,40 +25,5 @@ test_that("styler does not return error when there is no file to style", {
## ............................................................................
## highlighted region ####

test_that("styling active region works", {
dir <- tempfile("styler")
dir.create(dir)
region_path_temp <- file.path(dir, "addin_region-in.R")
region_path_perm <- testthat_file("public-api", "xyzaddin", "addin_region-in.R")
file.copy(region_path_perm, dir)

context <- structure(list(
id = "C533793B",
path = region_path_temp,
contents = c("fjkdsfa 2jy+wj/ 1+1 <?+d", ""),
selection = structure(list(structure(list(range = structure(list(
start = structure(
c(1, 17), .Names = c("row", "column"), class = "document_position"),
end = structure(c(1, 21), .Names = c("row", "column"),
class = "document_position")), .Names = c("start", "end"),
class = "document_range"), text = "1+1 "), .Names = c("range", "text"))),
.Names = "", class = "document_selection")),
.Names = c("id", "path", "contents", "selection"),
class = "document_context"
)

reference <- utf8::read_lines_enc(
testthat_file("public-api", "xyzaddin", "addin_region-out.R")
)

result <- expect_error(
mockr::with_mock(
get_rstudio_context = function() context,
style_active_region(),
.env = asNamespace("styler")
), NA)

after_styling <- utf8::read_lines_enc(region_path_temp)
expect_equivalent(after_styling, reference)
unlink(dir)
})
# styling active region cannot be tested automatically since
# rstudioapi::insertText() needs the context id.