New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

printf does not support \e (the escape character) #910

Closed
anordal opened this Issue Jul 11, 2013 · 2 comments

Comments

Projects
None yet
3 participants
@anordal
Contributor

anordal commented Jul 11, 2013

The "\e" escape sequence is recognised by at least bash versions of echo & printf, GNU coreutils /usr/bin/{echo,printf} and the GNU C compiler.

Now that printf suddenly became a builtin, it's important to not break people's scripts, or just not be worse than people are used to. Besides, the fish version of echo supports it!

I don't think "\e" is posix (dash and busybox don't support it), but when GNU standards deviate from posix, it's usually for a reason.

A patch that works for me:

diff --git a/builtin_printf.cpp b/builtin_printf.cpp
index e164f81..d5cf9e0 100644
--- a/builtin_printf.cpp
+++ b/builtin_printf.cpp
@@ -26,6 +26,7 @@
    \a = alert (bell)
    \b = backspace
    \c = produce no further output
+   \e = escape
    \f = form feed
    \n = new line
    \r = carriage return
@@ -319,6 +320,9 @@ void builtin_printf_state_t::print_esc_char(wchar_t c)
         case L'c':         /* Cancel the rest of the output. */
             this->early_exit = true;
             break;
+        case L'e':         /* Escape. */
+            this->append_output(L'\e');
+            break;
         case L'f':         /* Form feed. */
             this->append_output(L'\f');
             break;
@@ -369,7 +373,7 @@ long builtin_printf_state_t::print_esc(const wchar_t *escstart, bool octal_0)
             esc_value = esc_value * 8 + octal_to_bin(*p);
         this->append_format_output(L"%c", esc_value);
     }
-    else if (*p && wcschr(L"\"\\abcfnrtv", *p))
+    else if (*p && wcschr(L"\"\\abcefnrtv", *p))
         print_esc_char(*p++);
     else if (*p == L'u' || *p == L'U')
     {

(There shall be 3 tabs between case labels and comments, looks like they became spaces)

@xfix

This comment has been minimized.

Member

xfix commented Jul 12, 2013

It would be easier if this would be a pull request. Anyway, I've to say something, don't use \e, use \x1B. While gcc seems to allow \e, it's not standard C (or C++). I know it was considered to be in ANSI C, but it wasn't added, because it's only valid for ASCII (not that these days anybody cares about other encodings).

➜  tmp cat > e.cpp
#include <stdio.h>
int main() {
    printf("\e");
    return 0;
}
➜  tmp g++ -Wall -pedantic e.cpp
e.cpp: In function ‘int main()’:
e.cpp:3:12: warning: non-ISO-standard escape sequence, '\e' [enabled by default]

Otherwise, I think I'm fine with this change.

@ridiculousfish

This comment has been minimized.

Member

ridiculousfish commented Jul 16, 2013

Implemented here:
To git@github.com:fish-shell/fish-shell.git
cabebd9..d3bb2a7 master -> master

Thanks for reporting this, and suggesting a fix. And nice catch GlitchMr.

haarts pushed a commit to haarts/fish-shell that referenced this issue Nov 1, 2013

simnalamburt added a commit to simnalamburt/cgitc that referenced this issue Oct 28, 2016

Support built-in 'printf' function of fish 2.0.0
printf function of fish 2.0.0 does not support `\e` escaping.

See also:
  fish-shell/fish-shell#910
  fish-shell/fish-shell@d3bb2a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment