[RFC PATCH] Allow grouping of thousands in format string

[RFC PATCH] Allow grouping of thousands in format string

[Øystein Walle]

Putting an apostrophe before i, d, u, f, F, g or G is specified in POSIX
albeit noted as an extension to ISO C.
---

Bash handles this this and from what I can tell implements it much in the same
way so that stuff like %'s is passed straight on to the system's printf().
However POSIX also says that using the grouping specifier with other flags is
undefined. It works just fine on the limited range of systems I've tested it on
(all GNU).

If desired I could set a flag and then handle it further on by checking that
type == 2 or type == 3 or something like that, but that needs some rearranging.

If the idea behind the patch itself is undesired then I think the documentation
for zsh's printf should be updated.

 Src/builtin.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Src/builtin.c b/Src/builtin.c
index 614b17d..225c034 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3735,7 +3735,7 @@ bin_print(char *name, char **args, Options ops, int func)
     int nnl = 0, fmttrunc = 0, ret = 0, maxarg = 0, nc = 0;
     int flags[5], *len;
     char *start, *endptr, *c, *d, *flag, *buf = NULL, spec[13], *fmt = NULL;
-    char **first, **argp, *curarg, *flagch = "0+- #", save = '\0', nullstr = '\0';
+    char **first, **argp, *curarg, *flagch = "'0+- #", save = '\0', nullstr = '\0';
     size_t rcount, count = 0;
 #ifdef HAVE_OPEN_MEMSTREAM
     size_t mcount;
@@ -4312,8 +4312,8 @@ bin_print(char *name, char **args, Options ops, int func)
 		if (prec >= 0) *d++ = '.', *d++ = '*';
 	    }
 
-	    /* ignore any size modifier */
-	    if (*c == 'l' || *c == 'L' || *c == 'h') c++;
+	    /* ignore any size modifier and grouping specifier */
+	    if (*c == 'l' || *c == 'L' || *c == 'h' || *c == '\'') c++;
 
 	    if (!curarg && *argp) {
 		curarg = *argp;
-- 
2.2.0

Re: [RFC PATCH] Allow grouping of thousands in format string

[Oliver Kiddle]

Øystein Walle wrote:
> Putting an apostrophe before i, d, u, f, F, g or G is specified in POSIX
> albeit noted as an extension to ISO C.

There's other printf features that were left out because they weren't
portable at the time. %a %A and %F formats, for example.

After looking around the ' flag is supported on any system I now have
access to, even Solaris 8. However, I can't see a mention of it in the
OpenBSD manpage: http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/asprintf.3?query=printf&sec=3
Anyone have an OpenBSD system they can verify with? Even if it
doesn't support the flag, it'd be good to know if it is just ignored or
causes printf to fail. The flag is locale dependent anyway so being
ignored is quite reasonable for zsh's printf.

%a/A/F are in Solaris 10 which is a reasonable minimum to expect these
days. Otherwise, they need OpenBSD 4.5 though I've no idea how common
that still is. It'd be possible to test for them with autoconf but the
documentation would need to mention that they depend on the C library
implementing them.

> Bash handles this this and from what I can tell implements it much in the same
> way so that stuff like %'s is passed straight on to the system's printf().
> However POSIX also says that using the grouping specifier with other flags is
> undefined. It works just fine on the limited range of systems I've tested it on
> (all GNU).

Is it undefined with other "flags" or with non-numeric format
specifiers? It might be best to add a test case so we find out if the
behaviour is undesirable on some other system.

> If desired I could set a flag and then handle it further on by checking that
> type == 2 or type == 3 or something like that, but that needs some rearranging.

A flag is already set because it checks that flags aren't duplicated to
avoid overflowing the spec buffer. It wouldn't need much rearranging but
then it's not checking the type with other numeric only flags such as
+. For a more general implementation, the flags array could be replaced
by a single int and bit manipulation and it could set a bit mask for
compatible flags in the switch statement.

> int flags[5], *len;
>      char *start, *endptr, *c, *d, *flag, *buf = NULL, spec[13], *fmt = NULL;
> -    char **first, **argp, *curarg, *flagch = "0+- #", save = '\0', nullstr = '\0';
> +    char **first, **argp, *curarg, *flagch = "'0+- #", save = '\0', nullstr = '\0';

I'm fairly sure that you'll also need to increment the size of the flags
and spec arrays too.

> -	    /* ignore any size modifier */
> -	    if (*c == 'l' || *c == 'L' || *c == 'h') c++;
> +	    /* ignore any size modifier and grouping specifier */
> +	    if (*c == 'l' || *c == 'L' || *c == 'h' || *c == '\'') c++;

I'm not convinced this part is right. Can you remember why you needed
it? The ' flag comes before width specifiers and size modifiers.
In, e.g. printf "%2\$10f\n" 1 2
the ' flag comes after the $ and not before the f.

Oliver

Re: [RFC PATCH] Allow grouping of thousands in format string

[Jun T.]

2015/04/04 20:11, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:

> Anyone have an OpenBSD system they can verify with? Even if it
> doesn't support the flag, it'd be good to know if it is just ignored or
> causes printf to fail. The flag is locale dependent anyway so being
> ignored is quite reasonable for zsh's printf.

On OpenBSD 5.6, the ' flag (grouping) is silently ignored.
It has no effect but causes no error.

BTW, if we are going to use the patch by Øystein Walle, the array flags[]
defined at

builtin.c:3736:    int flags[5], *len;

must be flags[6]. (or allocate it by using sizeof(int)*strlen(flagch))