Skip to content

git_conflict_report() could be more informative #2003

@olivroy

Description

@olivroy

Pulls ideas from #1720

With git conflicts, it is never fun to have to scroll in a long file to find git conflicts location. When running pr_merge_main() for example, it could be more efficient to solve merge conflicts if usethis read the files and indicated where exactly merge conflicts happen. cli links allow you to jump to line. https://cli.r-lib.org/reference/links.html

For example, not mention conflicts of generated files and let the user know how to regenerate instead.

Here are a few examples:

  • README.md conflict should be solved with devtools::build_readme() (if README.Rmd exists)
  • Snapshot tests should be resolved via devtools::test()
  • man/ should be resolved via devtools::document() (if repo uses roxygen2)
  • Nudge which files were renamed. In my first version, a crash occured with a rename conflict.

Here is my PoC code to solve this:

My function isn't very effective since gert::git_conflicts() errors when renaming occured (r-lib/gert#229)

conflict_remove_renamed <- function(conflicts) {
  # TODO refine conflicts when I encounter renaming?
  renaming_occured <- conflicts$ancestor != conflicts$our | conflicts$our != conflicts$their
  if (!any(renaming_occured)) {
    return(conflicts)
  }
  # renamed files
  r <- conflicts[renaming_occured, , drop = FALSE]
  for (i in seq_len(nrow(renamed_files))) {
    cli::cli_inform(
      "Conflicts after renamed {.file {r$ancestor[i]}} to {.file {r$their[i]}}."
    )
  }

  # Return the conflicts df without renaming
  conflicts[!renaming_occured, , drop = FALSE]
}
conflict_remove_man <- function(conflicted_files) {
  # Check if any .Rd files changed
  exception_conflicts <- endsWith(conflicted_files, ".Rd")
  # only message is doc uses roxygen2.
  if (!any(exception_conflicts) || !usethis:::uses_roxygen()) {
    return(conflicted_files)
  }
  # Inform how to regenerate
  cli::cli_inform(c(
    i = "After fixing conflicts, run {.run devtools::document()} to fix conflicts in `man/`."
  ))
  # don't mention it in report
  conflicted_files[!exception_conflicts]
}
conflict_remove_snap <- function(conflicted_files) {
  # Check if any snapshot files changed
  exception_conflicts <- grepl("testthat/_snaps/.+\\.md", conflicted_files)
  # only message is doc uses roxygen2.
  if (!any(exception_conflicts)) {
    return(conflicted_files)
  }
  # Inform how to regenerate
  cli::cli_inform(c(
    i = "After fixing conflicts, run {.run devtools::test()} to update snapshots and fix conflicts."
  ))
  # don't mention it in report
  conflicted_files[!exception_conflicts]
}
conflict_remove_readme <- function(conflicted_files) {
  # Check if using README.Rmd first
  uses_readme_rmd <- !is.null(usethis:::path_first_existing(c("README.Rmd", "README.qmd")))
  if (!uses_readme_rmd) {
    return(conflicted_files)
  }

  # Check if README.md, README.Rmd (or README.qmd) changed
  exception_conflicts <- grepl("^README.[Rq]?md$", conflicted_files, ignore.case = TRUE)
  # return if no change in README
  if (!any(exception_conflicts)) {
    return(conflicted_files)
  }
  # Inform how to regenerate

  cli::cli_inform(c(
    i = "After fixing conflicts, run {.run devtools::build_readme()} to update {.path README.md} and fix conflicts."
  ))
  # don't mention generated file in report
  conflicted_files[conflicted_files != "README.md"]
}

# called for its side-effects of printing conflicts locations
# returns a named list of line numbers
git_conflicts_locations <- function(conflicted_files) {
  # set names
  # Only use existing files here (avoid deleted renamed files)
  existing_files <- conflicted_files[file.exists(conflicted_files)]
  names(existing_files) <- existing_files
  file_contents <- purrr::map(
    existing_files, function(x) readLines(x, warn = FALSE, encoding = "UTF-8")
  )
  locations <- purrr::map(
    file_contents,
    # only start location
    function(x) grep("^<<<<<<<", x)
    # To get all git locations
    # function(x) grep("^=======$|^<<<<<<<|^>>>>>>>", x)
  )
  locations_label <- purrr::imap_chr(
    locations,
    function(line, file) {
      if (length(line) == 1) {
        cli::format_inline("{.path {file}:{line[1]}}")
      } else {
        cli::format_inline("{.path {file}:{line[1]}} and {length(line) -1} other conflict{?s}")
      }
    }
  )
  cli::cli_bullets(rlang::set_names(locations_label, "*"))
  locations
}

git_conflict_report2 <- function() {
  conflicts <- gert::git_conflicts()
  if (nrow(conflicts) == 0L) {
    cli::cli_inform(c(
      "v" = "No git conflicts."
    ))
    return(invisible())
  }
  # message + remove renaming from report
  conflicts <- conflict_remove_renamed(conflicts)
  # FIXME complete crash in case of renaming conflicts.
  conflicted_files <- unlist(conflicts, use.names = FALSE)
  conflicted_files <- unique(conflicted_files)
  cli::cli_h2("Merge conflict report")
  # generated files remove and add reminder to regenarate
  conflicted_files <- conflict_remove_man(conflicted_files)
  conflicted_files <- conflict_remove_snap(conflicted_files)
  # only mention README.Rmd in conflicts if both README.Rmd and README.md conflict
  # Mention to build_readme()
  conflicted_files <- conflict_remove_readme(conflicted_files)
  # A list of file numbers, with each conflict line number start
  # prints the first conflict location of each file, by detecting <<<<<
  conflict_locations <- git_conflicts_locations(conflicted_files)
  invisible(conflicted_files)
}

image

usethis showcases this (with #1720):

image

I wrote this as standalone for now as I had to see a real-life merge conflict to do this, but with this base, I would happily harmonize it with how usethis does it.

  1. Adding the line numbers to the bulleted list
  2. Adding the generated files messages after accepting to merge. (i.e. if selecting option to solve file conflicts, indicate that user will have to regenerate man + update snapshot files, as it has to be clear that regenerating files should be the last thing to do.

For simplicity, I added the first conflict for each file, but could easily change that and inspire myself from spelling mistakes. (I have a PR there to make links to locations clickable ropensci/spelling#81)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gitgit, GitHub, and CI in general

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions