[U-Boot] [PATCH] common: cli_readline: Improve command line editing

[U-Boot] [PATCH] common: cli_readline: Improve command line editing

[James Byrne]

This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.

The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---

 common/cli_readline.c | 108 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 30 deletions(-)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..0d4ad49 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,94 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		 * handle standard linux xterm esc sequences for arrow key, etc.
 		 */
 		if (esc_len != 0) {
+			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act;
+
 			if (esc_len == 1) {
-				if (ichar == '[') {
-					esc_save[esc_len] = ichar;
-					esc_len = 2;
+				if (ichar == '[' || ichar == 'O')
+					act = ESC_SAVE;
+				else
+					act = ESC_REJECT;
+			} else if (esc_len == 2) {
+				switch (ichar) {
+				case 'D':	/* <- key */
+					ichar = CTL_CH('b');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^B handler */
+				case 'C':	/* -> key */
+					ichar = CTL_CH('f');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^F handler */
+				case 'H':	/* Home key */
+					ichar = CTL_CH('a');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^A handler */
+				case 'F':	/* End key */
+					ichar = CTL_CH('e');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^E handler */
+				case 'A':	/* up arrow */
+					ichar = CTL_CH('p');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^P handler */
+				case 'B':	/* down arrow */
+					ichar = CTL_CH('n');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^N handler */
+				case '1':
+				case '3':
+				case '4':
+				case '7':
+				case '8':
+					if (esc_save[1] == '[') {
+						/* see if next character is ~ */
+						act = ESC_SAVE;
+					} else {
+						act = ESC_REJECT;
+					}
+					break;
+				default:
+					act = ESC_REJECT;
+					break;
+				}
+			} else if (esc_len == 3) {
+				if (ichar == '~') {
+					switch (esc_save[2]) {
+					case '3':	/* Delete key */
+						ichar = CTL_CH('d');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^D handler */
+					case '1':	/* Home key */
+					case '7':
+						ichar = CTL_CH('a');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^A handler */
+					case '4':	/* End key */
+					case '8':
+						ichar = CTL_CH('e');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^E handler */
+					default:
+						act = ESC_REJECT;
+						break;
+					}
 				} else {
-					cread_add_str(esc_save, esc_len,
-						      insert, &num, &eol_num,
-						      buf, *len);
-					esc_len = 0;
+					act = ESC_REJECT;
 				}
-				continue;
 			}
 
-			switch (ichar) {
-			case 'D':	/* <- key */
-				ichar = CTL_CH('b');
-				esc_len = 0;
-				break;
-			case 'C':	/* -> key */
-				ichar = CTL_CH('f');
-				esc_len = 0;
-				break;	/* pass off to ^F handler */
-			case 'H':	/* Home key */
-				ichar = CTL_CH('a');
-				esc_len = 0;
-				break;	/* pass off to ^A handler */
-			case 'A':	/* up arrow */
-				ichar = CTL_CH('p');
-				esc_len = 0;
-				break;	/* pass off to ^P handler */
-			case 'B':	/* down arrow */
-				ichar = CTL_CH('n');
-				esc_len = 0;
-				break;	/* pass off to ^N handler */
-			default:
+			switch (act) {
+			case ESC_SAVE:
+				esc_save[esc_len++] = ichar;
+				continue;
+			case ESC_REJECT:
 				esc_save[esc_len++] = ichar;
 				cread_add_str(esc_save, esc_len, insert,
 					      &num, &eol_num, buf, *len);
 				esc_len = 0;
 				continue;
+			case ESC_CONVERTED:
+				esc_len = 0;
+				break;
 			}
 		}
 
-- 
1.9.1

[U-Boot] common: cli_readline: Improve command line editing

[Tom Rini]

On Wed, Jun 08, 2016 at 01:49:46PM +0100, James Byrne wrote:

> This improves the cread_line() function so that it will correctly
> process the 'Home', 'End', 'Delete' and arrow key escape sequences
> produced by various terminal emulators. This makes command line editing
> a more pleasant experience.
> 
> The previous code only supported the cursor keys and the 'Home' key, and
> only for certain terminal emulator configurations. This adds support for
> the 'End and 'Delete' keys, and recognises a wider range of escape
> sequences. For example, the left arrow key can be 'ESC O D' instead of
> 'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
> 'ESC 7 ~', depending on what terminal emulator you use and how it is
> configured.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>

This introduces a new warning:
common/cli_readline.c: In function ?cli_readline_into_buffer?:
common/cli_readline.c:361:4: warning: ?act? may be used uninitialized in this function [-Wmaybe-uninitialized]

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160714/ef2453f1/attachment.sig>

[U-Boot] [PATCH v2] common: cli_readline: Improve command line editing

[James Byrne]

This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.

The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---
Changes for v2
   - Explicitly initialize variable to avoid spurious compiler warning.

 common/cli_readline.c | 108 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 30 deletions(-)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..6690c13 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,94 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		 * handle standard linux xterm esc sequences for arrow key, etc.
 		 */
 		if (esc_len != 0) {
+			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act = ESC_REJECT;
+
 			if (esc_len == 1) {
-				if (ichar == '[') {
-					esc_save[esc_len] = ichar;
-					esc_len = 2;
+				if (ichar == '[' || ichar == 'O')
+					act = ESC_SAVE;
+				else
+					act = ESC_REJECT;
+			} else if (esc_len == 2) {
+				switch (ichar) {
+				case 'D':	/* <- key */
+					ichar = CTL_CH('b');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^B handler */
+				case 'C':	/* -> key */
+					ichar = CTL_CH('f');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^F handler */
+				case 'H':	/* Home key */
+					ichar = CTL_CH('a');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^A handler */
+				case 'F':	/* End key */
+					ichar = CTL_CH('e');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^E handler */
+				case 'A':	/* up arrow */
+					ichar = CTL_CH('p');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^P handler */
+				case 'B':	/* down arrow */
+					ichar = CTL_CH('n');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^N handler */
+				case '1':
+				case '3':
+				case '4':
+				case '7':
+				case '8':
+					if (esc_save[1] == '[') {
+						/* see if next character is ~ */
+						act = ESC_SAVE;
+					} else {
+						act = ESC_REJECT;
+					}
+					break;
+				default:
+					act = ESC_REJECT;
+					break;
+				}
+			} else if (esc_len == 3) {
+				if (ichar == '~') {
+					switch (esc_save[2]) {
+					case '3':	/* Delete key */
+						ichar = CTL_CH('d');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^D handler */
+					case '1':	/* Home key */
+					case '7':
+						ichar = CTL_CH('a');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^A handler */
+					case '4':	/* End key */
+					case '8':
+						ichar = CTL_CH('e');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^E handler */
+					default:
+						act = ESC_REJECT;
+						break;
+					}
 				} else {
-					cread_add_str(esc_save, esc_len,
-						      insert, &num, &eol_num,
-						      buf, *len);
-					esc_len = 0;
+					act = ESC_REJECT;
 				}
-				continue;
 			}
 
-			switch (ichar) {
-			case 'D':	/* <- key */
-				ichar = CTL_CH('b');
-				esc_len = 0;
-				break;
-			case 'C':	/* -> key */
-				ichar = CTL_CH('f');
-				esc_len = 0;
-				break;	/* pass off to ^F handler */
-			case 'H':	/* Home key */
-				ichar = CTL_CH('a');
-				esc_len = 0;
-				break;	/* pass off to ^A handler */
-			case 'A':	/* up arrow */
-				ichar = CTL_CH('p');
-				esc_len = 0;
-				break;	/* pass off to ^P handler */
-			case 'B':	/* down arrow */
-				ichar = CTL_CH('n');
-				esc_len = 0;
-				break;	/* pass off to ^N handler */
-			default:
+			switch (act) {
+			case ESC_SAVE:
+				esc_save[esc_len++] = ichar;
+				continue;
+			case ESC_REJECT:
 				esc_save[esc_len++] = ichar;
 				cread_add_str(esc_save, esc_len, insert,
 					      &num, &eol_num, buf, *len);
 				esc_len = 0;
 				continue;
+			case ESC_CONVERTED:
+				esc_len = 0;
+				break;
 			}
 		}
 
-- 
1.9.1

[U-Boot] [PATCH v2] common: cli_readline: Improve command line editing

[Tom Rini]

On Sun, Jul 31, 2016 at 10:58:15PM +0100, James Byrne wrote:

> This improves the cread_line() function so that it will correctly
> process the 'Home', 'End', 'Delete' and arrow key escape sequences
> produced by various terminal emulators. This makes command line editing
> a more pleasant experience.
> 
> The previous code only supported the cursor keys and the 'Home' key, and
> only for certain terminal emulator configurations. This adds support for
> the 'End and 'Delete' keys, and recognises a wider range of escape
> sequences. For example, the left arrow key can be 'ESC O D' instead of
> 'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
> 'ESC 7 ~', depending on what terminal emulator you use and how it is
> configured.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> ---
> Changes for v2
>    - Explicitly initialize variable to avoid spurious compiler warning.

Thanks.  But now:

> 
>  common/cli_readline.c | 108 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 78 insertions(+), 30 deletions(-)
> 
> diff --git a/common/cli_readline.c b/common/cli_readline.c
> index c1476e4..6690c13 100644
> --- a/common/cli_readline.c
> +++ b/common/cli_readline.c
> @@ -283,46 +283,94 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
>  		 * handle standard linux xterm esc sequences for arrow key, etc.
>  		 */
>  		if (esc_len != 0) {
> +			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act = ESC_REJECT;
> +
>  			if (esc_len == 1) {
> -				if (ichar == '[') {
> -					esc_save[esc_len] = ichar;
> -					esc_len = 2;
> +				if (ichar == '[' || ichar == 'O')
> +					act = ESC_SAVE;
> +				else
> +					act = ESC_REJECT;

Since act is ESC_REJECT to start with, we can kill the else.

> +			} else if (esc_len == 2) {
> +				switch (ichar) {
> +				case 'D':	/* <- key */
> +					ichar = CTL_CH('b');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^B handler */
> +				case 'C':	/* -> key */
> +					ichar = CTL_CH('f');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^F handler */
> +				case 'H':	/* Home key */
> +					ichar = CTL_CH('a');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^A handler */
> +				case 'F':	/* End key */
> +					ichar = CTL_CH('e');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^E handler */
> +				case 'A':	/* up arrow */
> +					ichar = CTL_CH('p');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^P handler */
> +				case 'B':	/* down arrow */
> +					ichar = CTL_CH('n');
> +					act = ESC_CONVERTED;
> +					break;	/* pass off to ^N handler */
> +				case '1':
> +				case '3':
> +				case '4':
> +				case '7':
> +				case '8':
> +					if (esc_save[1] == '[') {
> +						/* see if next character is ~ */
> +						act = ESC_SAVE;
> +					} else {
> +						act = ESC_REJECT;
> +					}

And here too.

> +					break;
> +				default:
> +					act = ESC_REJECT;
> +					break;

And then we can drop the default case too.

> +				}
> +			} else if (esc_len == 3) {
> +				if (ichar == '~') {
> +					switch (esc_save[2]) {
> +					case '3':	/* Delete key */
> +						ichar = CTL_CH('d');
> +						act = ESC_CONVERTED;
> +						break;	/* pass to ^D handler */
> +					case '1':	/* Home key */
> +					case '7':
> +						ichar = CTL_CH('a');
> +						act = ESC_CONVERTED;
> +						break;	/* pass to ^A handler */
> +					case '4':	/* End key */
> +					case '8':
> +						ichar = CTL_CH('e');
> +						act = ESC_CONVERTED;
> +						break;	/* pass to ^E handler */
> +					default:
> +						act = ESC_REJECT;
> +						break;

Same.

> +					}
>  				} else {
> -					cread_add_str(esc_save, esc_len,
> -						      insert, &num, &eol_num,
> -						      buf, *len);
> -					esc_len = 0;
> +					act = ESC_REJECT;
>  				}

And this is also redundant.

Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160731/5f3d0572/attachment.sig>

[U-Boot] [PATCH v3] common: cli_readline: Improve command line editing

[James Byrne]

This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.

The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
Changes for v2
   - Explicitly initialize variable to avoid spurious compiler warning.
Changes for v3
   - Remove unnecessary setting of 'act' to ESC_REJECT (now its default
     value).

---
 common/cli_readline.c | 98 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 31 deletions(-)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..ecded11 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,82 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		 * handle standard linux xterm esc sequences for arrow key, etc.
 		 */
 		if (esc_len != 0) {
+			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act = ESC_REJECT;
+
 			if (esc_len == 1) {
-				if (ichar == '[') {
-					esc_save[esc_len] = ichar;
-					esc_len = 2;
-				} else {
-					cread_add_str(esc_save, esc_len,
-						      insert, &num, &eol_num,
-						      buf, *len);
-					esc_len = 0;
+				if (ichar == '[' || ichar == 'O')
+					act = ESC_SAVE;
+			} else if (esc_len == 2) {
+				switch (ichar) {
+				case 'D':	/* <- key */
+					ichar = CTL_CH('b');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^B handler */
+				case 'C':	/* -> key */
+					ichar = CTL_CH('f');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^F handler */
+				case 'H':	/* Home key */
+					ichar = CTL_CH('a');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^A handler */
+				case 'F':	/* End key */
+					ichar = CTL_CH('e');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^E handler */
+				case 'A':	/* up arrow */
+					ichar = CTL_CH('p');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^P handler */
+				case 'B':	/* down arrow */
+					ichar = CTL_CH('n');
+					act = ESC_CONVERTED;
+					break;	/* pass off to ^N handler */
+				case '1':
+				case '3':
+				case '4':
+				case '7':
+				case '8':
+					if (esc_save[1] == '[') {
+						/* see if next character is ~ */
+						act = ESC_SAVE;
+					}
+					break;
+				}
+			} else if (esc_len == 3) {
+				if (ichar == '~') {
+					switch (esc_save[2]) {
+					case '3':	/* Delete key */
+						ichar = CTL_CH('d');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^D handler */
+					case '1':	/* Home key */
+					case '7':
+						ichar = CTL_CH('a');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^A handler */
+					case '4':	/* End key */
+					case '8':
+						ichar = CTL_CH('e');
+						act = ESC_CONVERTED;
+						break;	/* pass to ^E handler */
+					}
 				}
-				continue;
 			}
 
-			switch (ichar) {
-			case 'D':	/* <- key */
-				ichar = CTL_CH('b');
-				esc_len = 0;
-				break;
-			case 'C':	/* -> key */
-				ichar = CTL_CH('f');
-				esc_len = 0;
-				break;	/* pass off to ^F handler */
-			case 'H':	/* Home key */
-				ichar = CTL_CH('a');
-				esc_len = 0;
-				break;	/* pass off to ^A handler */
-			case 'A':	/* up arrow */
-				ichar = CTL_CH('p');
-				esc_len = 0;
-				break;	/* pass off to ^P handler */
-			case 'B':	/* down arrow */
-				ichar = CTL_CH('n');
-				esc_len = 0;
-				break;	/* pass off to ^N handler */
-			default:
+			switch (act) {
+			case ESC_SAVE:
+				esc_save[esc_len++] = ichar;
+				continue;
+			case ESC_REJECT:
 				esc_save[esc_len++] = ichar;
 				cread_add_str(esc_save, esc_len, insert,
 					      &num, &eol_num, buf, *len);
 				esc_len = 0;
 				continue;
+			case ESC_CONVERTED:
+				esc_len = 0;
+				break;
 			}
 		}
 
-- 
1.9.1

[U-Boot] [U-Boot, v3] common: cli_readline: Improve command line editing

[Tom Rini]

On Tue, Aug 16, 2016 at 06:16:28PM +0100, James Byrne wrote:

> This improves the cread_line() function so that it will correctly
> process the 'Home', 'End', 'Delete' and arrow key escape sequences
> produced by various terminal emulators. This makes command line editing
> a more pleasant experience.
> 
> The previous code only supported the cursor keys and the 'Home' key, and
> only for certain terminal emulator configurations. This adds support for
> the 'End and 'Delete' keys, and recognises a wider range of escape
> sequences. For example, the left arrow key can be 'ESC O D' instead of
> 'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
> 'ESC 7 ~', depending on what terminal emulator you use and how it is
> configured.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> Changes for v2
>    - Explicitly initialize variable to avoid spurious compiler warning.
> Changes for v3
>    - Remove unnecessary setting of 'act' to ESC_REJECT (now its default
>      value).

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160821/292ce255/attachment.sig>