Re: i18n/l10n summary

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

Thanks for that patch: it's a good move forward for i18n. Some suggestions:

* Today I fixed the bug with "%%" and the 'error' function, so there's no need 
for a FIXME or a workaround any more.

* In strings.texi, reorder the format spec description so that it matches the 
textual order of a format spec. This should lessen confusion.

* Allow field numbers in a %% spec. All other components of a format spec are 
allowed in %%, so odd to report an error for just field numbers.

* There is no need for a special diagnostic for field numbers greater than 
PTRDIFF_MAX. Just use the same diagnostic other too-large field numbers use. 
This avoids a need for an alloca.

* Reword "Invalid field number `0'" to "Invalid format field number 0" to make 
it more obvious that it's a format and there's no need to quote the 0.

Proposed further patch attached (it addresses the above points), along with a 
copy of your patch rebased to current master for convenience.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Implement-field-numbers-in-format-strings.patch --]
[-- Type: text/x-patch; name="0001-Implement-field-numbers-in-format-strings.patch", Size: 10666 bytes --]

From 7f98d9c6d59472abf189ccd5a5c76415f9d1cbb3 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Wed, 31 May 2017 22:09:39 -0700
Subject: [PATCH 1/2] Implement field numbers in format strings

A field number explicitly specifies the argument to be formatted.
This is especially important for potential localization work, since
grammars of various languages dictate different word orders.

* src/editfns.c (Fformat): Update documentation.
(styled_format): Implement field numbers.

* doc/lispref/strings.texi (Formatting Strings): Document field numbers.

* lisp/emacs-lisp/bytecomp.el (byte-compile-format-warn): Adapt.

* test/src/editfns-tests.el (format-with-field): New unit test.
---
 doc/lispref/strings.texi    | 31 ++++++++++++++++++++++---
 etc/NEWS                    |  3 +++
 lisp/emacs-lisp/bytecomp.el | 11 ++++++---
 src/editfns.c               | 55 ++++++++++++++++++++++++++++++++++++++-------
 test/src/editfns-tests.el   | 18 +++++++++++++++
 5 files changed, 104 insertions(+), 14 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 9436a96..9bf52f2 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -864,7 +864,8 @@ Formatting Strings
   (format "%s" @var{arbitrary-string})
 @end example
 
-  If @var{string} contains more than one format specification, the
+  If @var{string} contains more than one format specification and none
+of the format specifications contain an explicit field number, the
 format specifications correspond to successive values from
 @var{objects}.  Thus, the first format specification in @var{string}
 uses the first such value, the second format specification uses the
@@ -961,6 +962,25 @@ Formatting Strings
 @end group
 @end example
 
+@cindex field number
+  A specification can have a @dfn{field number}, which is a decimal
+number after the initial @samp{%}, followed by a literal dollar sign
+@samp{$}.  If you provide a field numer, then the argument to be
+printed corresponds to the given field number instead of the next
+argument.  Field numbers start at 1.
+
+You can mix specifications with and without field numbers.  A
+specification without a field number that follows a specification with
+a field number will convert the argument after the one specified by
+the field number:
+
+@example
+(format "First argument %2$s, then %s, then %1$s" 1 2 3)
+     @result{} "First argument 2, then 3, then 1"
+@end example
+
+You can't use field numbers in a @samp{%%} specification.
+
 @cindex field width
 @cindex padding
   A specification can have a @dfn{width}, which is a decimal number
@@ -996,9 +1016,14 @@ Formatting Strings
 @end group
 @end example
 
+If you want to use both a field number and a width, place the field
+number before the width.  For example, in @samp{%2$7s}, @samp{2} is
+the field number and @samp{7} is the width.
+
 @cindex flags in format specifications
-  Immediately after the @samp{%} and before the optional width
-specifier, you can also put certain @dfn{flag characters}.
+  After the @samp{%} and before the optional width specifier, you can
+also put certain @dfn{flag characters}.  The flag characters need to
+come directly after a potential field number.
 
   The flag @samp{+} inserts a plus sign before a positive number, so
 that it always has a sign.  A space character as flag inserts a space
diff --git a/etc/NEWS b/etc/NEWS
index 43e7897..e31c0c7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -364,6 +364,9 @@ large integers from being displayed as characters.
 ** Two new commands for finding the source code of Emacs Lisp
 libraries: 'find-library-other-window' and 'find-library-other-frame'.
 
+** You can now provide explicit field numbers in format specifiers.
+For example, '(format "%2$s %1$s" 1 2)' produces "2 1".
+
 \f
 * Editing Changes in Emacs 26.1
 
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 12a7d4a..e5b9b47 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1375,10 +1375,15 @@ byte-compile-format-warn
     (let ((nfields (with-temp-buffer
 		     (insert (nth 1 form))
 		     (goto-char (point-min))
-		     (let ((n 0))
+		     (let ((i 0) (n 0))
 		       (while (re-search-forward "%." nil t)
-			 (unless (eq ?% (char-after (1+ (match-beginning 0))))
-			   (setq n (1+ n))))
+                         (backward-char)
+			 (unless (eq ?% (char-after))
+                           (setq i (if (looking-at "\\([0-9]+\\)\\$")
+                                       (string-to-number (match-string 1) 10)
+                                     (1+ i))
+                                 n (max n i)))
+                         (forward-char))
 		       n)))
 	  (nargs (- (length form) 2)))
       (unless (= nargs nfields)
diff --git a/src/editfns.c b/src/editfns.c
index 89a6724..44341ce 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -48,6 +48,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <float.h>
 #include <limits.h>
 
+#include <c-ctype.h>
 #include <intprops.h>
 #include <stdlib.h>
 #include <strftime.h>
@@ -3856,7 +3857,7 @@ The first argument is a format control string.
 The other arguments are substituted into it to make the result, a string.
 
 The format control string may contain %-sequences meaning to substitute
-the next available argument:
+the next available argument, or the argument explicitly specified:
 
 %s means print a string argument.  Actually, prints any object, with `princ'.
 %d means print as signed number in decimal.
@@ -3873,13 +3874,17 @@ the next available argument:
 The argument used for %d, %o, %x, %e, %f, %g or %c must be a number.
 Use %% to put a single % into the output.
 
-A %-sequence may contain optional flag, width, and precision
-specifiers, as follows:
+A %-sequence may contain optional field number, flag, width, and
+precision specifiers, as follows:
 
-  %<flags><width><precision>character
+  %<field><flags><width><precision>character
 
-where flags is [+ #-0]+, width is [0-9]+, and precision is a literal
-period "." followed by [0-9]+
+where field is [0-9]+ followed by a literal dollar "$", flags is
+[+ #-0]+, width is [0-9]+, and precision is a literal period "."
+followed by [0-9]+.
+
+If field is given, it must be a one-based argument number; the given
+argument is substituted instead of the next one.
 
 The + flag character inserts a + before any positive number, while a
 space inserts a space before any positive number; these flags only
@@ -4032,14 +4037,19 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	{
 	  /* General format specifications look like
 
-	     '%' [flags] [field-width] [precision] format
+	     '%' [field-number] [flags] [field-width] [precision] format
 
 	     where
 
+             field-number ::= [0-9]+ '$'
 	     flags ::= [-+0# ]+
 	     field-width ::= [0-9]+
 	     precision ::= '.' [0-9]*
 
+             If a field-number is specified, it specifies the argument
+             number to substitute.  Otherwise, the next argument is
+             taken.
+
 	     If a field-width is specified, it specifies to which width
 	     the output should be padded with blanks, if the output
 	     string is shorter than field-width.
@@ -4048,6 +4058,29 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	     digits to print after the '.' for floats, or the max.
 	     number of chars to print from a string.  */
 
+          char *field_end;
+          uintmax_t raw_field = strtoumax (format, &field_end, 10);
+          bool has_field = false;
+          if (c_isdigit (*format) && *field_end == '$')
+            {
+              if (raw_field < 1 || raw_field >= PTRDIFF_MAX)
+                {
+                  /* doprnt doesn't support %.*s, so we need to copy
+                     the field number string.  */
+                  ptrdiff_t length = field_end - format;
+                  eassert (length > 0);
+                  eassert (length < PTRDIFF_MAX);
+                  char *field = SAFE_ALLOCA (length + 1);
+                  memcpy (field, format, length);
+                  field[length] = '\0';
+                  error ("Invalid field number `%s'", field);
+                }
+              has_field = true;
+              /* n is incremented below.  */
+              n = raw_field - 1;
+              format = field_end + 1;
+            }
+
 	  bool minus_flag = false;
 	  bool  plus_flag = false;
 	  bool space_flag = false;
@@ -4090,7 +4123,13 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	  memset (&discarded[format0 - format_start], 1,
 		  format - format0 - (conversion == '%'));
 	  if (conversion == '%')
-	    goto copy_char;
+            {
+              if (has_field)
+                /* FIXME: `error' doesn't appear to support `%%'.  */
+                error ("Field number specified together with `%c' conversion",
+                       '%');
+              goto copy_char;
+            }
 
 	  ++n;
 	  if (! (n < nargs))
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index 8019eb0..f76c6c9 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -177,4 +177,22 @@ transpose-test-get-byte-positions
            (format-time-string "%Y-%m-%d %H:%M:%S.%3N %z" nil
                                (concat (make-string 2048 ?X) "0")))))
 
+(ert-deftest format-with-field ()
+  (should (equal (format "First argument %2$s, then %s, then %1$s" 1 2 3)
+                 "First argument 2, then 3, then 1"))
+  (should (equal (format "a %2$s %d %1$d %2$S %d %d b" 11 "22" 33 44)
+                 "a 22 33 11 \"22\" 33 44 b"))
+  (should (equal (format "a %08$s %s b" 1 2 3 4 5 6 7 8 9) "a 8 9 b"))
+  (should (equal (should-error (format "a %999999$s b" 11))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %$s b" 11))
+                 ;; FIXME: there shouldn't be two % in the error
+                 ;; string!
+                 '(error "Invalid format operation %%$")))
+  (should (equal (should-error (format "a %0$s b" 11))
+                 '(error "Invalid field number `0'")))
+  (should (equal
+           (should-error (format "a %1$% %s b" 11))
+           '(error "Field number specified together with `%' conversion"))))
+
 ;;; editfns-tests.el ends here
-- 
2.9.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Minor-improvements-to-format-field-numbers.patch --]
[-- Type: text/x-patch; name="0002-Minor-improvements-to-format-field-numbers.patch", Size: 12995 bytes --]

From 6b7edbf14be8b18e0781559345edda7346a1e363 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 31 May 2017 22:09:39 -0700
Subject: [PATCH 2/2] Minor improvements to format field numbers

* src/editfns.c (styled_format): Allow field numbers in a %% spec.
No need for a special diagnostic for field numbers greater than
PTRDIFF_MAX.  Reword diagnostic for field 0.
* test/src/editfns-tests.el (format-with-field): Adjust to match.
---
 doc/lispref/strings.texi  | 119 +++++++++++++++++++++-------------------------
 etc/NEWS                  |   2 +-
 src/editfns.c             |  56 ++++++++--------------
 test/src/editfns-tests.el |  10 ++--
 4 files changed, 79 insertions(+), 108 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 9bf52f2..7577410 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -864,15 +864,6 @@ Formatting Strings
   (format "%s" @var{arbitrary-string})
 @end example
 
-  If @var{string} contains more than one format specification and none
-of the format specifications contain an explicit field number, the
-format specifications correspond to successive values from
-@var{objects}.  Thus, the first format specification in @var{string}
-uses the first such value, the second format specification uses the
-second such value, and so on.  Any extra format specifications (those
-for which there are no corresponding values) cause an error.  Any
-extra values to be formatted are ignored.
-
   Certain format specifications require values of particular types.  If
 you supply a value that doesn't fit the requirements, an error is
 signaled.
@@ -962,68 +953,33 @@ Formatting Strings
 @end group
 @end example
 
+  By default, format specifications correspond to successive values from
+@var{objects}.  Thus, the first format specification in @var{string}
+uses the first such value, the second format specification uses the
+second such value, and so on.  Any extra format specifications (those
+for which there are no corresponding values) cause an error.  Any
+extra values to be formatted are ignored.
+
 @cindex field number
-  A specification can have a @dfn{field number}, which is a decimal
-number after the initial @samp{%}, followed by a literal dollar sign
-@samp{$}.  If you provide a field numer, then the argument to be
-printed corresponds to the given field number instead of the next
-argument.  Field numbers start at 1.
+  A format specification can have a @dfn{field number}, which is a
+decimal number immediately after the initial @samp{%}, followed by a
+literal dollar sign @samp{$}.  It causes the format specification to
+convert the argument with the given number instead of the next
+argument.  Argument 1 is the argument just after the format.
 
-You can mix specifications with and without field numbers.  A
+  You can mix specifications with and without field numbers.  A
 specification without a field number that follows a specification with
 a field number will convert the argument after the one specified by
 the field number:
 
 @example
-(format "First argument %2$s, then %s, then %1$s" 1 2 3)
-     @result{} "First argument 2, then 3, then 1"
-@end example
-
-You can't use field numbers in a @samp{%%} specification.
-
-@cindex field width
-@cindex padding
-  A specification can have a @dfn{width}, which is a decimal number
-between the @samp{%} and the specification character.  If the printed
-representation of the object contains fewer characters than this
-width, @code{format} extends it with padding.  The width specifier is
-ignored for the @samp{%%} specification.  Any padding introduced by
-the width specifier normally consists of spaces inserted on the left:
-
-@example
-(format "%5d is padded on the left with spaces" 123)
-     @result{} "  123 is padded on the left with spaces"
-@end example
-
-@noindent
-If the width is too small, @code{format} does not truncate the
-object's printed representation.  Thus, you can use a width to specify
-a minimum spacing between columns with no risk of losing information.
-In the following two examples, @samp{%7s} specifies a minimum width
-of 7.  In the first case, the string inserted in place of @samp{%7s}
-has only 3 letters, and needs 4 blank spaces as padding.  In the
-second case, the string @code{"specification"} is 13 letters wide but
-is not truncated.
-
-@example
-@group
-(format "The word '%7s' has %d letters in it."
-        "foo" (length "foo"))
-     @result{} "The word '    foo' has 3 letters in it."
-(format "The word '%7s' has %d letters in it."
-        "specification" (length "specification"))
-     @result{} "The word 'specification' has 13 letters in it."
-@end group
+(format "Argument %2$s, then %s, then %1$s" "x" "y" "z")
+     @result{} "Argument y, then z, then x"
 @end example
 
-If you want to use both a field number and a width, place the field
-number before the width.  For example, in @samp{%2$7s}, @samp{2} is
-the field number and @samp{7} is the width.
-
 @cindex flags in format specifications
-  After the @samp{%} and before the optional width specifier, you can
-also put certain @dfn{flag characters}.  The flag characters need to
-come directly after a potential field number.
+  After the @samp{%} and any field number, you can put certain
+@dfn{flag characters}.
 
   The flag @samp{+} inserts a plus sign before a positive number, so
 that it always has a sign.  A space character as flag inserts a space
@@ -1048,8 +1004,8 @@ Formatting Strings
 These specification characters accept the @samp{0} flag, but still pad
 with @emph{spaces}.
 
-  The flag @samp{-} causes the padding inserted by the width
-specifier, if any, to be inserted on the right rather than the left.
+  The flag @samp{-} causes any padding inserted by the width,
+if specified, to be inserted on the right rather than the left.
 If both @samp{-} and @samp{0} are present, the @samp{0} flag is
 ignored.
 
@@ -1067,9 +1023,44 @@ Formatting Strings
 @end group
 @end example
 
+@cindex field width
+@cindex padding
+  A specification can have a @dfn{width}, which is a decimal number
+that appears after any field number and flags.  If the printed
+representation of the object contains fewer characters than this
+width, @code{format} extends it with padding.  The width is
+ignored for the @samp{%%} specification.  Any padding introduced by
+the width normally consists of spaces inserted on the left:
+
+@example
+(format "%5d is padded on the left with spaces" 123)
+     @result{} "  123 is padded on the left with spaces"
+@end example
+
+@noindent
+If the width is too small, @code{format} does not truncate the
+object's printed representation.  Thus, you can use a width to specify
+a minimum spacing between columns with no risk of losing information.
+In the following two examples, @samp{%7s} specifies a minimum width
+of 7.  In the first case, the string inserted in place of @samp{%7s}
+has only 3 letters, and needs 4 blank spaces as padding.  In the
+second case, the string @code{"specification"} is 13 letters wide but
+is not truncated.
+
+@example
+@group
+(format "The word '%7s' has %d letters in it."
+        "foo" (length "foo"))
+     @result{} "The word '    foo' has 3 letters in it."
+(format "The word '%7s' has %d letters in it."
+        "specification" (length "specification"))
+     @result{} "The word 'specification' has 13 letters in it."
+@end group
+@end example
+
 @cindex precision in format specifications
   All the specification characters allow an optional @dfn{precision}
-before the character (after the width, if present).  The precision is
+after the field number, flags and width, if present.  The precision is
 a decimal-point @samp{.} followed by a digit-string.  For the
 floating-point specifications (@samp{%e} and @samp{%f}), the
 precision specifies how many digits following the decimal point to
diff --git a/etc/NEWS b/etc/NEWS
index e31c0c7..dfe81cd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -365,7 +365,7 @@ large integers from being displayed as characters.
 libraries: 'find-library-other-window' and 'find-library-other-frame'.
 
 ** You can now provide explicit field numbers in format specifiers.
-For example, '(format "%2$s %1$s" 1 2)' produces "2 1".
+For example, '(format "%2$s %1$s" "X" "Y")' produces "Y X".
 
 \f
 * Editing Changes in Emacs 26.1
diff --git a/src/editfns.c b/src/editfns.c
index 44341ce..98187df 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4046,9 +4046,8 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	     field-width ::= [0-9]+
 	     precision ::= '.' [0-9]*
 
-             If a field-number is specified, it specifies the argument
-             number to substitute.  Otherwise, the next argument is
-             taken.
+	     If present, a field-number specifies the argument number
+	     to substitute.  Otherwise, the next argument is taken.
 
 	     If a field-width is specified, it specifies to which width
 	     the output should be padded with blanks, if the output
@@ -4058,28 +4057,20 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	     digits to print after the '.' for floats, or the max.
 	     number of chars to print from a string.  */
 
-          char *field_end;
-          uintmax_t raw_field = strtoumax (format, &field_end, 10);
-          bool has_field = false;
-          if (c_isdigit (*format) && *field_end == '$')
-            {
-              if (raw_field < 1 || raw_field >= PTRDIFF_MAX)
-                {
-                  /* doprnt doesn't support %.*s, so we need to copy
-                     the field number string.  */
-                  ptrdiff_t length = field_end - format;
-                  eassert (length > 0);
-                  eassert (length < PTRDIFF_MAX);
-                  char *field = SAFE_ALLOCA (length + 1);
-                  memcpy (field, format, length);
-                  field[length] = '\0';
-                  error ("Invalid field number `%s'", field);
-                }
-              has_field = true;
-              /* n is incremented below.  */
-              n = raw_field - 1;
-              format = field_end + 1;
-            }
+	  uintmax_t num;
+	  char *num_end;
+	  if (c_isdigit (*format))
+	    {
+	      num = strtoumax (format, &num_end, 10);
+	      if (*num_end == '$')
+		{
+		  if (num == 0)
+		    error ("Invalid format field number 0");
+		  n = min (num, PTRDIFF_MAX);
+		  n--;
+		  format = num_end + 1;
+		}
+	    }
 
 	  bool minus_flag = false;
 	  bool  plus_flag = false;
@@ -4104,11 +4095,10 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	  space_flag &= ! plus_flag;
 	  zero_flag &= ! minus_flag;
 
-	  char *num_end;
-	  uintmax_t raw_field_width = strtoumax (format, &num_end, 10);
-	  if (max_bufsize <= raw_field_width)
+	  num = strtoumax (format, &num_end, 10);
+	  if (max_bufsize <= num)
 	    string_overflow ();
-	  ptrdiff_t field_width = raw_field_width;
+	  ptrdiff_t field_width = num;
 
 	  bool precision_given = *num_end == '.';
 	  uintmax_t precision = (precision_given
@@ -4123,13 +4113,7 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	  memset (&discarded[format0 - format_start], 1,
 		  format - format0 - (conversion == '%'));
 	  if (conversion == '%')
-            {
-              if (has_field)
-                /* FIXME: `error' doesn't appear to support `%%'.  */
-                error ("Field number specified together with `%c' conversion",
-                       '%');
-              goto copy_char;
-            }
+	    goto copy_char;
 
 	  ++n;
 	  if (! (n < nargs))
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index f76c6c9..c5923aa 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -186,13 +186,9 @@ transpose-test-get-byte-positions
   (should (equal (should-error (format "a %999999$s b" 11))
                  '(error "Not enough arguments for format string")))
   (should (equal (should-error (format "a %$s b" 11))
-                 ;; FIXME: there shouldn't be two % in the error
-                 ;; string!
-                 '(error "Invalid format operation %%$")))
+                 '(error "Invalid format operation %$")))
   (should (equal (should-error (format "a %0$s b" 11))
-                 '(error "Invalid field number `0'")))
-  (should (equal
-           (should-error (format "a %1$% %s b" 11))
-           '(error "Field number specified together with `%' conversion"))))
+                 '(error "Invalid format field number 0")))
+  (should (equal (format "a %1$% %s b" 11) "a % 11 b")))
 
 ;;; editfns-tests.el ends here
-- 
2.9.4