Skip to content
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.

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

Improve HTML column width rendering #1744

Merged
merged 13 commits into from
Jul 3, 2024

Conversation

rich-iannone
Copy link
Member

@rich-iannone rich-iannone commented Jul 3, 2024

This addresses the issue where having a row group set as a column in the stub corrupts the setting of column widths (as <col> items) in the <colgroup> tag. The problem is that the for the row-group-as-column is not added but a real column is present. This makes the number of <col> tag not match the total number of columns.

An additional (small) problem is that we don't have a row_group() helper that could be used in cols_width() (we have stub()). However, we can use the column name that is assigned for row groups.

Fixes: #1253
Fixes: #1510

Also, ensure that ordering of columns in matches real ordering (row_group first, stub second, everything else after that).
@olivroy
Copy link
Collaborator

olivroy commented Jul 3, 2024

As an fyi, the weird test-coverage failures are due to snapshot changes. Locally, you can delete the culprit snapshot failures. (r-lib/testthat#1646)

Locally, you can delete the current files, re-run the tests and examine the diff to see what went wrong (or if the changes are actually an improvement)!

@rich-iannone
Copy link
Member Author

^^^ that's my workflow exactly. For checking diffs, I'm using diffchecker.com (can't find a better way in GH or GH Desktop)

image

@olivroy
Copy link
Collaborator

olivroy commented Jul 3, 2024

I created a function that triggers diffviewer + git show

Here is what I shared in r-lib/diffviewer#4 (comment)

  # * RStudio not providing an easy view to preview + delete untracked files rstudio/rstudio#12453, rstudio/rstudio#9310
  # * gert not providing git show r-lib/gert#219
  # * git pane sometimes, not very ergonomic rstudio/rstudio#11669
  git_visual_diff <- function(uncommited_file = rstudioapi::getSourceEditorContext()$path, repo = ".") {
    rlang::check_installed(c("diffviewer", "gert", "xfun", "fs"))
    if (is.null(uncommited_file)) {
      cli::cli_abort("Not for unsaved documents.")
    }
    uncommited_file <- fs::path_rel(
      fs::path_real(uncommited_file),
      fs::path_real(repo)
    )
    if (stringr::str_detect(uncommited_file, "\\.\\./")) {
      cli::cli_abort("uncommited file can't be outside repo.")
    }
    if (!fs::is_file(uncommited_file)) {
      cli::cli_abort("{.arg uncommited file} must be an existing file.")
    }
    if (is.na(gert::git_stat_files(uncommited_file)$modified)) {
      # Solution
      cli::cli_inform("New (uncommited) file {.run [Click to delete])(fs::file_delete({as.character(uncommited_file)}))}.")
      tmp_dir <- withr::local_tempdir()
      empty_file <- fs::path(tmp_dir, uncommited_file)
      xfun::write_utf8(
        text = "",
        empty_file
      )
      return(diffviewer::visual_diff(
        file_old = empty_file,
        file_new = uncommited_file,
        height = 100
      ))
    }
    if (repo != ".") {
      # change dir temporarily
      withr::with_dir(repo, {
        old_file_lines <- system(paste("git show HEAD:", uncommited_file), intern = TRUE)
      })
    } else {
      old_file_lines <- system(paste0("git show HEAD:", uncommited_file, ""), intern = TRUE)
    }

    tmp_dir <- withr::local_tempdir()
    file_old <- fs::path(tmp_dir, paste0(fs::path_ext_remove(fs::path_file(uncommited_file)), "-uncommited"), ext = fs::path_ext(uncommited_file))
    xfun::write_utf8(
      con = file_old,
      text = old_file_lines
    )
    diffviewer::visual_diff(
      file_old = file_old,
      file_new = uncommited_file,
      height = 100
    )
  }

@rich-iannone
Copy link
Member Author

If you wanted to, you could add this function to a new file in the scripts directory :)

@rich-iannone rich-iannone merged commit 785063d into master Jul 3, 2024
12 checks passed
@rich-iannone rich-iannone deleted the improve-html-column-width-rendering branch July 3, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants