From f5e79d57b7b50515fd0d5846b76122433e8d5f30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kirill=20M=C3=BCller?= <krlmlr@users.noreply.github.com>
Date: Tue, 9 Jan 2024 04:41:00 +0100
Subject: [PATCH] More careful ordering for `@family` tag (#1563)

---
 NEWS.md                            |  2 +
 R/rd-family.R                      |  7 ++-
 R/util-locale.R                    |  8 ++-
 man/tags-index-crossref.Rd         |  2 +-
 man/tags-reuse.Rd                  |  4 +-
 tests/testthat/_snaps/rd-family.md | 86 ++++++++++++++++++++++++++++++
 tests/testthat/test-rd-family.R    | 24 +++++++++
 7 files changed, 127 insertions(+), 6 deletions(-)
 create mode 100644 tests/testthat/_snaps/rd-family.md

diff --git a/NEWS.md b/NEWS.md
index 5d63d603e..144b7930e 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -62,6 +62,8 @@
 
 * `@inherit` can now also inherit from `@format` (#1293). 
 
+* `@family` lists are now ordered more carefully, "foo1" comes after "foo" (#1563, @krlmlr).
+
 # roxygen2 7.2.3
 
 * roxygen2 now supports HTML blocks in markdown. They are only included
diff --git a/R/rd-family.R b/R/rd-family.R
index 3b93013a2..df0365443 100644
--- a/R/rd-family.R
+++ b/R/rd-family.R
@@ -51,12 +51,15 @@ topics_process_family <- function(topics, env) {
       if (length(others) < 1)
         next
 
-      by_file <- map_chr(aliases[others], function(x) {
+      other_aliases <- aliases[others]
+      other_aliases_order <- map_chr(other_aliases, function(x) escape(x[1]))
+
+      by_file <- map_chr(other_aliases[order_c(other_aliases_order)], function(x) {
         obj <- find_object(x[1], env)
         suffix <- if (is.function(obj$value)) "()" else ""
         paste0("\\code{\\link{", escape(x[1]), "}", suffix, "}")
       })
-      links <- paste(sort_c(by_file), collapse = ",\n")
+      links <- paste(by_file, collapse = ",\n")
       seealso <- topics_process_family_prefix(family)
 
       topic$add(rd_section("seealso", paste0(seealso, "\n", links)))
diff --git a/R/util-locale.R b/R/util-locale.R
index fa324ce8f..7360aabfb 100644
--- a/R/util-locale.R
+++ b/R/util-locale.R
@@ -11,4 +11,10 @@ with_collate <- function(locale, code) {
   force(code)
 }
 
-sort_c <- function(x, ...) with_collate("C", sort(x, ...))
+sort_c <- function(x, ...) {
+  with_collate("C", sort(x, ...))
+}
+
+order_c <- function(x, ...) {
+  with_collate("C", order(x, ...))
+}
diff --git a/man/tags-index-crossref.Rd b/man/tags-index-crossref.Rd
index 265e93bca..b8e4aec48 100644
--- a/man/tags-index-crossref.Rd
+++ b/man/tags-index-crossref.Rd
@@ -40,8 +40,8 @@ Other less frequently used tags:
 }
 \seealso{
 Other documentation tags: 
-\code{\link{tags-rd-other}},
 \code{\link{tags-rd}},
+\code{\link{tags-rd-other}},
 \code{\link{tags-reuse}}
 }
 \concept{documentation tags}
diff --git a/man/tags-reuse.Rd b/man/tags-reuse.Rd
index d3aa3c3bc..de062a563 100644
--- a/man/tags-reuse.Rd
+++ b/man/tags-reuse.Rd
@@ -56,7 +56,7 @@ Other less frequently used tags:
 \seealso{
 Other documentation tags: 
 \code{\link{tags-index-crossref}},
-\code{\link{tags-rd-other}},
-\code{\link{tags-rd}}
+\code{\link{tags-rd}},
+\code{\link{tags-rd-other}}
 }
 \concept{documentation tags}
diff --git a/tests/testthat/_snaps/rd-family.md b/tests/testthat/_snaps/rd-family.md
new file mode 100644
index 000000000..915bba09a
--- /dev/null
+++ b/tests/testthat/_snaps/rd-family.md
@@ -0,0 +1,86 @@
+# careful ordering
+
+    Code
+      out
+    Output
+      $foo1.Rd
+      % Generated by roxygen2: do not edit by hand
+      % Please edit documentation in ./<text>
+      \name{foo1}
+      \alias{foo1}
+      \title{foo1}
+      \usage{
+      foo1()
+      }
+      \description{
+      foo1
+      }
+      \seealso{
+      Other a: 
+      \code{\link{Foo3}()},
+      \code{\link{foo}()},
+      \code{\link{foo2}()}
+      }
+      \concept{a}
+      
+      $foo2.Rd
+      % Generated by roxygen2: do not edit by hand
+      % Please edit documentation in ./<text>
+      \name{foo2}
+      \alias{foo2}
+      \title{foo2}
+      \usage{
+      foo2()
+      }
+      \description{
+      foo2
+      }
+      \seealso{
+      Other a: 
+      \code{\link{Foo3}()},
+      \code{\link{foo}()},
+      \code{\link{foo1}()}
+      }
+      \concept{a}
+      
+      $Foo3.Rd
+      % Generated by roxygen2: do not edit by hand
+      % Please edit documentation in ./<text>
+      \name{Foo3}
+      \alias{Foo3}
+      \title{Foo3}
+      \usage{
+      Foo3()
+      }
+      \description{
+      Foo3
+      }
+      \seealso{
+      Other a: 
+      \code{\link{foo}()},
+      \code{\link{foo1}()},
+      \code{\link{foo2}()}
+      }
+      \concept{a}
+      
+      $foo.Rd
+      % Generated by roxygen2: do not edit by hand
+      % Please edit documentation in ./<text>
+      \name{foo}
+      \alias{foo}
+      \title{foo}
+      \usage{
+      foo()
+      }
+      \description{
+      foo
+      }
+      \seealso{
+      Other a: 
+      \code{\link{Foo3}()},
+      \code{\link{foo1}()},
+      \code{\link{foo2}()}
+      }
+      \concept{a}
+      
+
diff --git a/tests/testthat/test-rd-family.R b/tests/testthat/test-rd-family.R
index d6e8eacf3..fb7b8b591 100644
--- a/tests/testthat/test-rd-family.R
+++ b/tests/testthat/test-rd-family.R
@@ -123,3 +123,27 @@ test_that("custom family prefixes can be set", {
 
   expect_match(out$get_value("seealso"), "^Custom prefix:")
 })
+
+test_that("careful ordering", {
+  out <- roc_proc_text(rd_roclet(), "
+    #' foo1
+    #' @family a
+    foo1 <- function() {}
+
+    #' foo2
+    #' @family a
+    foo2 <- function() {}
+
+    #' Foo3
+    #' @family a
+    Foo3 <- function() {}
+
+    #' foo
+    #' @family a
+    foo <- function() {}
+  ")
+
+  expect_snapshot({
+    out
+  })
+})