From 0bde698f819b9352ca246a3b435a72593f50405b Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Sun, 17 Mar 2019 16:33:58 +0100
Subject: [PATCH] printf: Don't die on incomplete conversions

POSIX dictates here that incomplete conversions, like in

    printf %d\n 15.2

or

    printf %d 14g

are still printed along with any error.

This seems alright, as it allows users to silence stderr to accept incomplete conversions.

This commit implements it, but what's a bit weird is the ordering between stdout and stderr,
causing the error to be printed _after_, like

    15
    14
    15.1: value not completely converted
    14,2: value not completely converted

but that seems like a general issue with how we buffer the streams.

(I know that nonfatal_error is a copy of most of fatal_error - I tried
differently, and va_* is weird)

Fixes #5532.
---
 CHANGELOG.md           |  1 +
 src/builtin_printf.cpp | 27 +++++++++++++++++++++++----
 tests/printf.err       |  1 +
 tests/printf.in        |  4 ++++
 tests/printf.out       |  3 +++
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b92b7ef24..a476b3fb9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@
 - `math` now accepts `--scale=max` for the maximum scale (#5579).
 - `complete --do-complete` now also does fuzzy matches (#5467).
 - `count` now also counts lines fed on stdin (#5744).
+- `printf` prints what it can when input hasn't been fully converted to a number, but still prints an error (#5532).
 
 ### Interactive improvements
 - Major improvements in performance and functionality to the 'sorin' sample prompt (#5411).
diff --git a/src/builtin_printf.cpp b/src/builtin_printf.cpp
index d12d9c596..09b3a9283 100644
--- a/src/builtin_printf.cpp
+++ b/src/builtin_printf.cpp
@@ -90,8 +90,10 @@ struct builtin_printf_state_t {
 
     int print_formatted(const wchar_t *format, int argc, wchar_t **argv);
 
+    void nonfatal_error(const wchar_t *fmt, ...);
     void fatal_error(const wchar_t *format, ...);
 
+
     long print_esc(const wchar_t *escstart, bool octal_0);
     void print_esc_string(const wchar_t *str);
     void print_esc_char(wchar_t c);
@@ -193,6 +195,22 @@ static int octal_to_bin(wchar_t c) {
     }
 }
 
+void builtin_printf_state_t::nonfatal_error(const wchar_t *fmt, ...) {
+    // Don't error twice.
+    if (early_exit) return;
+
+    va_list va;
+    va_start(va, fmt);
+    wcstring errstr = vformat_string(fmt, va);
+    va_end(va);
+    streams.err.append(errstr);
+    if (!string_suffixes_string(L"\n", errstr)) streams.err.push_back(L'\n');
+
+    // We set the exit code to error, because one occured,
+    // but we don't do an early exit so we still print what we can.
+    this->exit_code = STATUS_CMD_ERROR;
+}
+
 void builtin_printf_state_t::fatal_error(const wchar_t *fmt, ...) {
     // Don't error twice.
     if (early_exit) return;
@@ -207,7 +225,6 @@ void builtin_printf_state_t::fatal_error(const wchar_t *fmt, ...) {
     this->exit_code = STATUS_CMD_ERROR;
     this->early_exit = true;
 }
-
 void builtin_printf_state_t::append_output(wchar_t c) {
     // Don't output if we're done.
     if (early_exit) return;
@@ -241,10 +258,12 @@ void builtin_printf_state_t::verify_numeric(const wchar_t *s, const wchar_t *end
             this->fatal_error(L"%ls: %s", s, std::strerror(errcode));
         }
     } else if (*end) {
-        if (s == end)
+        if (s == end) {
             this->fatal_error(_(L"%ls: expected a numeric value"), s);
-        else
-            this->fatal_error(_(L"%ls: value not completely converted"), s);
+        } else {
+            // This isn't entirely fatal - the value should still be printed.
+            this->nonfatal_error(_(L"%ls: value not completely converted"), s);
+        }
     }
 }
 
diff --git a/tests/printf.err b/tests/printf.err
index 40686c8f9..d8d4de233 100644
--- a/tests/printf.err
+++ b/tests/printf.err
@@ -1,2 +1,3 @@
 2,34: value not completely converted
 0xABCDEF12345678901: Number out of range
+15.1: value not completely converted
diff --git a/tests/printf.in b/tests/printf.in
index 9c8b9f9a8..d7174253e 100644
--- a/tests/printf.in
+++ b/tests/printf.in
@@ -72,3 +72,7 @@ printf 'long hex4 %X\n' 0xABCDEF12345678901
 printf 'long decimal %d\n' 498216206594
 printf 'long signed %d\n' -498216206595
 printf 'long signed to unsigned %u\n' -498216206596
+
+# Verify numeric conversion still happens even if it couldn't be fully converted
+printf '%d\n' 15.1
+echo $status
diff --git a/tests/printf.out b/tests/printf.out
index b965be413..40f19f791 100644
--- a/tests/printf.out
+++ b/tests/printf.out
@@ -16,6 +16,7 @@ a
 0000000 376
 0000001
 1.230000e+00
+2.000000e+00
 3,450000e+00
 4,560000e+00
 long hex1 73ffffff9a
@@ -24,3 +25,5 @@ long hex3 ABCDEF1234567890
 long hex4 long decimal 498216206594
 long signed -498216206595
 long signed to unsigned 18446743575493345020
+15
+1