From 0e7b171b950f8c302fa31ee55ef7337468edbc93 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Sun, 27 Oct 2019 07:54:06 -0500
Subject: [PATCH 1/2] Change minor_breaks interface

To pass in both limits and major breaks. This will make it much easier to create useful families of minor break functions in scales.

Fixes #3583
---
 R/scale-.r              | 14 +++++++++++---
 man/continuous_scale.Rd |  3 ++-
 man/scale_continuous.Rd |  3 ++-
 man/scale_gradient.Rd   |  3 ++-
 man/scale_size.Rd       |  3 ++-
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/R/scale-.r b/R/scale-.r
index 76e32d006d..2e2f99b534 100644
--- a/R/scale-.r
+++ b/R/scale-.r
@@ -24,7 +24,8 @@
 #'   - `waiver()` for the default breaks (one minor break between
 #'     each major break)
 #'   - A numeric vector of positions
-#'   - A function that given the limits returns a vector of minor breaks.
+#'   - A function passed the limits and major breaks, which should return a
+#'     vector of minor breaks.
 #' @param labels One of:
 #'   - `NULL` for no labels
 #'   - `waiver()` for the default labels computed by the
@@ -613,8 +614,15 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale,
         breaks <- self$trans$minor_breaks(b, limits, n)
       }
     } else if (is.function(self$minor_breaks)) {
-      # Find breaks in data space, and convert to numeric
-      breaks <- self$minor_breaks(self$trans$inverse(limits))
+      breaks_fun <- self$minor_breaks
+
+      if (length(formals(breaks_fun)) == 1) {
+        # Old API just gets limits
+        breaks <- breaks_fun(self$trans$inverse(limits))
+      } else {
+        # New API gets limits and breaks
+        breaks <- breaks_fun(self$trans$inverse(limits), b)
+      }
       breaks <- self$trans$transform(breaks)
     } else {
       breaks <- self$trans$transform(self$minor_breaks)
diff --git a/man/continuous_scale.Rd b/man/continuous_scale.Rd
index 6c5ee2a3fb..a7ca942947 100644
--- a/man/continuous_scale.Rd
+++ b/man/continuous_scale.Rd
@@ -41,7 +41,8 @@ as output (e.g., a function returned by \code{\link[scales:extended_breaks]{scal
 \item \code{waiver()} for the default breaks (one minor break between
 each major break)
 \item A numeric vector of positions
-\item A function that given the limits returns a vector of minor breaks.
+\item A function passed the limits and major breaks, which should return a
+vector of minor breaks.
 }}
 
 \item{labels}{One of:
diff --git a/man/scale_continuous.Rd b/man/scale_continuous.Rd
index c853b8c83d..fbbc3343e6 100644
--- a/man/scale_continuous.Rd
+++ b/man/scale_continuous.Rd
@@ -57,7 +57,8 @@ as output (e.g., a function returned by \code{\link[scales:extended_breaks]{scal
 \item \code{waiver()} for the default breaks (one minor break between
 each major break)
 \item A numeric vector of positions
-\item A function that given the limits returns a vector of minor breaks.
+\item A function passed the limits and major breaks, which should return a
+vector of minor breaks.
 }}
 
 \item{labels}{One of:
diff --git a/man/scale_gradient.Rd b/man/scale_gradient.Rd
index d59e93fff7..397269e60c 100644
--- a/man/scale_gradient.Rd
+++ b/man/scale_gradient.Rd
@@ -69,7 +69,8 @@ as output (e.g., a function returned by \code{\link[scales:extended_breaks]{scal
 \item \code{waiver()} for the default breaks (one minor break between
 each major break)
 \item A numeric vector of positions
-\item A function that given the limits returns a vector of minor breaks.
+\item A function passed the limits and major breaks, which should return a
+vector of minor breaks.
 }}
   \item{labels}{One of:
 \itemize{
diff --git a/man/scale_size.Rd b/man/scale_size.Rd
index 31c659c4e7..e9b3000050 100644
--- a/man/scale_size.Rd
+++ b/man/scale_size.Rd
@@ -116,7 +116,8 @@ as output (e.g., a function returned by \code{\link[scales:extended_breaks]{scal
 \item \code{waiver()} for the default breaks (one minor break between
 each major break)
 \item A numeric vector of positions
-\item A function that given the limits returns a vector of minor breaks.
+\item A function passed the limits and major breaks, which should return a
+vector of minor breaks.
 }}
   \item{labels}{One of:
 \itemize{

From 52c9ec019bb81501d0a094f7d4e1dc47acde54d6 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Sun, 27 Oct 2019 08:51:40 -0500
Subject: [PATCH 2/2] Test with real input

---
 R/scale-.r | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/R/scale-.r b/R/scale-.r
index 2e2f99b534..b14d445826 100644
--- a/R/scale-.r
+++ b/R/scale-.r
@@ -614,14 +614,19 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale,
         breaks <- self$trans$minor_breaks(b, limits, n)
       }
     } else if (is.function(self$minor_breaks)) {
-      breaks_fun <- self$minor_breaks
+      # Need to reach inside of ggproto method to get actual function
+      # This is fine because it's not a method, it's a function supplied by the
+      # user that is assigned into the ggproto object on creation
+      breaks_fun <- environment(self$minor_breaks)$f
 
+      # Limits are on transformed scale, so we need to back transform
+      # so breaks function can work in original data space
       if (length(formals(breaks_fun)) == 1) {
         # Old API just gets limits
         breaks <- breaks_fun(self$trans$inverse(limits))
       } else {
         # New API gets limits and breaks
-        breaks <- breaks_fun(self$trans$inverse(limits), b)
+        breaks <- breaks_fun(self$trans$inverse(limits), self$trans$inverse(b))
       }
       breaks <- self$trans$transform(breaks)
     } else {