diff --git a/NEWS.md b/NEWS.md index 1a3f466..9b1db4a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # reuseme (development version) +* `complete_todo()` no longer deletes the full line. It only deletes what it says it deletes (#27). + * `file_outline()` works better with news files and headings at the end of files. * `file_outline()` gives a better error for empty paths. diff --git a/R/outline.R b/R/outline.R index 77ac4b0..e23ebb8 100644 --- a/R/outline.R +++ b/R/outline.R @@ -592,7 +592,8 @@ construct_outline_link <- function(.data, is_saved_doc, dir_common, pattern) { "- {.run [Done{cli::symbol$tick}?](reuseme::complete_todo(", # Removed ending dot. (possibly will fail with older versions) .data$line_id[cn], ", '", .data$file[cn], "', '", - stringr::str_sub(stringr::str_replace_all(.data$content[cn], "'|\\{|\\}|\\)|\\(|\\[\\]|\\+", "."), start = -15L), "'))}", + # modify regex twice if needed (see below) + stringr::str_sub(stringr::str_replace_all(.data$content[cn], "\\^|\\$|'|\\{|\\}|\\)|\\(|\\[\\]|\\+", "."), start = -15L), "'))}", .data$rs_version[cn] ) # truncate other elements @@ -610,7 +611,9 @@ construct_outline_link <- function(.data, is_saved_doc, dir_common, pattern) { outline_el, "- {.run [Done{cli::symbol$tick}?](reuseme::complete_todo(", # Removed ending dot. (possibly will fail with older versions) - line_id, ", '", file, "', '", stringr::str_sub(stringr::str_replace_all(content, "'|\\{|\\}|\\)|\\(|\\[\\]|\\+", "."), start = -15L), "'))}", + + # modify regex twice if needed (see above) + line_id, ", '", file, "', '", stringr::str_sub(stringr::str_replace_all(content, "\\^|\\$|'|\\{|\\}|\\)|\\(|\\[\\]|\\+", "."), start = -15L), "'))}", rs_version ), outline_el2 diff --git a/R/use-todo.R b/R/use-todo.R index d87a676..017dbe7 100644 --- a/R/use-todo.R +++ b/R/use-todo.R @@ -133,37 +133,44 @@ complete_todo <- function(line_id, file, regexp, rm_line = NULL) { )) } + line_content_before_comment <- stringr::str_extract(line_content, "([^#]+)#", group = 1) if (is.null(rm_line)) { - rm_line <- tag_type %in% c("TODO", "FIXME") + rm_line <- tag_type %in% c("TODO", "FIXME") # && is.na(line_content_before_comment) } + line_content_todo <- stringr::str_extract(line_content, "#[^#]+") line_content_show <- stringr::str_squish(line_content) if (rm_line) { - line_content_show <- cli::style_strikethrough(line_content_show) + if (is.na(line_content_before_comment)) { + line_content_show <- cli::style_strikethrough(line_content_show) + } else { + line_content_show <- paste(line_content_before_comment, cli::style_strikethrough(line_content_todo)) + } } else { # Only strikethrough the tag - regex <- paste0("\\s", tag_type) + regex <- paste0(" ", tag_type) regex_new <- cli::style_strikethrough(regex) line_content_show <- stringr::str_replace(line_content_show, regex, regex_new) } - + file_line <- paste0(file, ":", line_id) cli::cli_alert_success( - "Removed {.code {line_content_show}} from {.file {file}}!" + "Removed {.code {line_content_show}} from {.file {file_line}}!" ) + # Rem + line_content_new <- strip_todo_line(line_content, only_rm_tag = !rm_line) - if (rm_line) { - file_content_new <- file_content[-line_id] - line_content_new <- "" - } else { - line_content_new <- sub( - pattern = paste0(tag_type, "\\s+"), - replacement = "", - line_content - ) - + if (nzchar(line_content_new)) { file_content[line_id] <- line_content_new file_content_new <- file_content + } else { + file_content_new <- file_content[-line_id] + if (!rm_line) { + # WIll remove this line eventually + # remove line if it ends up empty. not supposed to happen + cli::cli_abort("This should not happen. We were supposed not to remove lines", .internal = TRUE) + } } + if (length(file_content_new) > 0) { usethis::write_over(path = file, lines = file_content_new, overwrite = TRUE, quiet = TRUE) } else { @@ -236,3 +243,25 @@ compute_path_todo <- function(todo, proj) { list(path_todo = path_todo, todo = todo) } # this doesn't work + +# accepts a single line +strip_todo_line <- function(x, only_rm_tag = FALSE) { + check_string(x) + if (!stringr::str_detect(x, "TODO|WORK|FIXME")) { + cli::cli_abort("Could not detect a todo tag in x") + } + if (only_rm_tag) { + x_new <- stringr::str_remove(x, "\\s(TODO|WORK|FIXME)") + } else { + x_new <- stringr::str_extract(x, "([^#]+)\\#+", group = 1) + if (is.na(x_new)) { + x_new <- "" + # cli::cli_abort("Could not extract content before tag") + } + } + if (x_new == x) { + cli::cli_abort("Could not make any change to x = {.val {x}}") + } + x_new <- stringr::str_trim(x_new, "right") + x_new +} diff --git a/tests/testthat/test-use-todo.R b/tests/testthat/test-use-todo.R index e2af578..7024b2f 100644 --- a/tests/testthat/test-use-todo.R +++ b/tests/testthat/test-use-todo.R @@ -17,12 +17,23 @@ test_that("Marking TODO as done detects tags", { ) }) +test_that("todo items are correctly stripped", { + expect_equal( + strip_todo_line("2^2 # TODO this"), + "2^2" + ) + expect_equal( + strip_todo_line("2^2 # TODO this", only_rm_tag = TRUE), + "2^2 # this" + ) +}) + test_that("Marking a TODO item as done works", { tmp <- tempfile(fileext = "R") content <- c( "# I Want this done", - "# TODO item to delete", - "# WORK Explain what the next code does.", + "2^2 # TODO item to delete", + "2^2 # WORK Explain what the next code does.", "# TODO with {.href [cli hyperlinks](https://cli.r-lib.org/reference/links.html)}", "# FIXME Repair this function", " # TODO Check r-lib/usethis#1890", @@ -48,7 +59,7 @@ test_that("Marking a TODO item as done works", { expect_complete_todo( out <- complete_todo( - line_id = 3, + line_id = 4, file = tmp, regexp = "Explain what the next code does" ), @@ -56,7 +67,7 @@ test_that("Marking a TODO item as done works", { ) expect_equal( out, - "# Explain what the next code does." + "2^2 # Explain what the next code does." ) expect_complete_todo( out <- complete_todo( @@ -70,7 +81,8 @@ test_that("Marking a TODO item as done works", { read_utf8(tmp), c( "# I Want this done", - "# Explain what the next code does.", + "2^2", + "2^2 # Explain what the next code does.", "# TODO with {.href [cli hyperlinks](https://cli.r-lib.org/reference/links.html)}", "# FIXME Repair this function", "# TODO Check https://github.com/r-lib/usethis/issues/1890",