emulation: Revise the extended color escape sequences

Add support for true color sequences according to ITU-T T.416,
i.e. CSI 38:2:[color_space_id]:R:G:B[:more_params]m. Color space id
and further parameters are ignored.

Keep support for the misinterpreted CSI 38:2:R:G:Bm format (missing
color space id) for now, to be dropped at some point in the future.

Keep support for 256-color sequences according to ITU-T T.416,
i.e. CSI 38:5:INDEXm. Allow and ignore further parameters.

Keep support for the de facto standard CSI 38;2;R;G;Bm and CSI 38;5;INDEXm
supported by most terminal emulators and emitted by most apps.

Drop support for mixed use of semicolons and colons.

Add command line flags to perf/256test.sh and perf/img.sh to select the
emitted sequences, switch to the de jure format as default.

https://bugzilla.gnome.org/show_bug.cgi?id=791456

4 files changed, 68 insertions, 47 deletions

diff --git a/perf/256test.sh b/perf/256test.sh
index 53b8d7f9..051ac457 100755
--- a/perf/256test.sh
+++ b/perf/256test.sh
@@ -17,6 +17,22 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
+sep=':'
+if [ "$1" = "-colon" -o "$1" = "-official" -o "$1" = "-dejure" ]; then
+  shift
+elif [ "$1" = "-semicolon" -o "$1" = "-common" -o "$1" = "-defacto" ]; then
+  sep=';'
+  shift
+fi
+
+if [ $# != 0 ]; then
+  echo 'Usage: 256test.sh [-format]' >&2
+  echo >&2
+  echo '  -colon|-official|-dejure:     Official format (default)  \e[38:5:INDEXm' >&2
+  echo '  -semicolon|-common|-defacto:  Commonly used format       \e[38;5;INDEXm' >&2
+  exit 1
+fi
+
 format_number() {
   local c=$'\u254F'
   if [ $1 -lt 10 ]; then
@@ -55,17 +71,17 @@ allcolors() {
   echo "-- 8 bright colors: SGR ${2}0..${2}7 --"
   somecolors 0 7 "$2"
   echo
-  echo "-- 256 colors: SGR ${1}8;5;0..255 --"
-  somecolors 0 15 "${1}8;5;"
+  echo "-- 256 colors: SGR ${1}8${sep}5${sep}0..255 --"
+  somecolors 0 15 "${1}8${sep}5${sep}"
   echo
-  somecolors  16  51 "${1}8;5;"
-  somecolors  52  87 "${1}8;5;"
-  somecolors  88 123 "${1}8;5;"
-  somecolors 124 159 "${1}8;5;"
-  somecolors 160 195 "${1}8;5;"
-  somecolors 196 231 "${1}8;5;"
+  somecolors  16  51 "${1}8${sep}5${sep}"
+  somecolors  52  87 "${1}8${sep}5${sep}"
+  somecolors  88 123 "${1}8${sep}5${sep}"
+  somecolors 124 159 "${1}8${sep}5${sep}"
+  somecolors 160 195 "${1}8${sep}5${sep}"
+  somecolors 196 231 "${1}8${sep}5${sep}"
   echo
-  somecolors 232 255 "${1}8;5;"
+  somecolors 232 255 "${1}8${sep}5${sep}"
 }
 
 allcolors 3 9
diff --git a/perf/img.sh b/perf/img.sh
index bae95605..60de5187 100755
--- a/perf/img.sh
+++ b/perf/img.sh
@@ -17,8 +17,25 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-if [ $# != 1 -o "$1" = "--help" ]; then
-  echo 'Usage: img.sh imagefile' >&2
+sep1=':'
+sep2='::'
+if [ "$1" = "-colon4" -o "$1" = "-official" -o "$1" = "-dejure" ]; then
+  shift
+elif [ "$1" = "-colon3" -o "$1" = "-wrong" ]; then
+  sep2=':'
+  shift
+elif [ "$1" = "-semicolon" -o "$1" = "-common" -o "$1" = "-defacto" ]; then
+  sep1=';'
+  sep2=';'
+  shift
+fi
+
+if [ $# != 1 -o "${1:0:1}" = "-" ]; then
+  echo 'Usage: img.sh [-format] imagefile' >&2
+  echo >&2
+  echo '  -colon4|-official|-dejure:    Official format (default)  \e[38:2::R:G:Bm' >&2
+  echo '  -colon3|-wrong:               Misinterpreted format      \e[38:2:R:G:Bm' >&2
+  echo '  -semicolon|-common|-defacto:  Commonly used format       \e[38;2;R;G;Bm' >&2
   exit 1
 elif [ -z $(type -p convert) ]; then
   echo 'Please install ImageMagick to run this script.' >&2
@@ -41,16 +58,16 @@ while IFS=',:() ' read col row dummy red green blue rest; do
   fi
 
   if [ $((row%2)) = 0 ]; then
-    upper[$col]="$red;$green;$blue"
+    upper[$col]="$red$sep1$green$sep1$blue"
   else
-    lower[$col]="$red;$green;$blue"
+    lower[$col]="$red$sep1$green$sep1$blue"
   fi
 
   # After reading every second image row, print them out.
   if [ $((row%2)) = 1 -a $col = $((COLUMNS-1)) ]; then
     i=0
     while [ $i -lt $COLUMNS ]; do
-      echo -ne "\\e[38;2;${upper[$i]};48;2;${lower[$i]}m▀"
+      echo -ne "\\e[38${sep1}2${sep2}${upper[$i]};48${sep1}2${sep2}${lower[$i]}m▀"
       i=$((i+1))
     done
     # \e[K is useful when you resize the terminal while this script is still running.
@@ -64,7 +81,7 @@ done
 if [ "${upper[0]}" != "" ]; then
   i=0
   while [ $i -lt $COLUMNS ]; do
-    echo -ne "\\e[38;2;${upper[$i]}m▀"
+    echo -ne "\\e[38${sep1}2${sep2}${upper[$i]}m▀"
     i=$((i+1))
   done
   echo -e "\\e[0m\e[K"
diff --git a/src/vteinternal.hh b/src/vteinternal.hh
index 21d3e7cf..2b9e7ba2 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -1322,7 +1322,8 @@ public:
         inline void erase_characters(long count);
         inline void insert_blank_character();
         inline int32_t parse_sgr_38_48_parameters(vte::parser::Params const& params,
-                                                  unsigned int *index);
+                                                  unsigned int *index,
+                                                  bool might_contain_color_space_id);
         inline void move_cursor_backward(vte::grid::column_t columns);
         inline void move_cursor_forward(vte::grid::column_t columns);
         inline void move_cursor_tab();
diff --git a/src/vteseq.cc b/src/vteseq.cc
index 58791b95..ba180146 100644
--- a/src/vteseq.cc
+++ b/src/vteseq.cc
@@ -1798,11 +1798,16 @@ VteTerminalPrivate::seq_vertical_tab(vte::parser::Params const& params)
 }
 
 /* Parse parameters of SGR 38, 48 or 58, starting at @index within @params.
+ * If @might_contain_color_space_id, a true color sequence sequence is started, and after
+ * its leading number "2" at least 4 more parameters are present, then there's an (ignored)
+ * color_space_id before the three color components. See the comment below in
+ * seq_character_attributes() to understand the different accepted formats.
  * Returns the color index, or -1 on error.
  * Increments @index to point to the last consumed parameter (not beyond). */
 int32_t
 VteTerminalPrivate::parse_sgr_38_48_parameters(vte::parser::Params const& params,
-                                               unsigned int *index)
+                                               unsigned int *index,
+                                               bool might_contain_color_space_id)
 {
         auto n_params = params.size();
         if (*index < n_params) {
@@ -1814,6 +1819,8 @@ VteTerminalPrivate::parse_sgr_38_48_parameters(vte::parser::Params const& params
                 case 2: {
                         if (G_UNLIKELY(*index + 3 >= n_params))
 				return -1;
+                        if (might_contain_color_space_id && *index + 5 <= n_params)
+			        *index += 1;
 
                         long param1, param2, param3;
                         if (G_UNLIKELY(!params.number_at_unchecked(*index + 1, param1) ||
@@ -1870,10 +1877,7 @@ VteTerminalPrivate::seq_character_attributes(vte::parser::Params const& params)
                         case 58:
                         {
                                 unsigned int index = 1;
-                                auto color = parse_sgr_38_48_parameters(subparams, &index);
-                                /* Bail out on additional colon-separated values. */
-                                if (G_UNLIKELY(index != subparams.size() - 1))
-                                        continue;
+                                auto color = parse_sgr_38_48_parameters(subparams, &index, true);
                                 if (G_LIKELY (color != -1)) {
                                         if (param0 == 38) {
                                                 m_defaults.attr.fore = color;
@@ -1963,35 +1967,18 @@ VteTerminalPrivate::seq_character_attributes(vte::parser::Params const& params)
 		{
 			/* The format looks like:
 			 * - 256 color indexed palette:
-			 *   - ^[[38;5;INDEXm
-			 *   - ^[[38;5:INDEXm
-			 *   - ^[[38:5:INDEXm
+                         *   - ^[[38:5:INDEXm  (de jure standard: ITU-T T.416 / ISO/IEC 8613-6; we also allow and ignore further parameters)
+                         *   - ^[[38;5;INDEXm  (de facto standard, understood by probably all terminal emulators that support 256 colors)
 			 * - true colors:
-			 *   - ^[[38;2;RED;GREEN;BLUEm
-			 *   - ^[[38;2:RED:GREEN:BLUEm
-			 *   - ^[[38:2:RED:GREEN:BLUEm
-			 * See bug 685759 for details.
-			 * The fully colon versions were handled above separately. The code is reached
-			 * if the first separator is a semicolon. */
+                         *   - ^[[38:2:[id]:RED:GREEN:BLUE[:...]m  (de jure standard: ITU-T T.416 / ISO/IEC 8613-6)
+                         *   - ^[[38:2:RED:GREEN:BLUEm             (common misinterpretation of the standard, FIXME: stop supporting it at some point)
+                         *   - ^[[38;2;RED;GREEN;BLUEm             (de facto standard, understood by probably all terminal emulators that support true colors)
+                         * See bugs 685759 and 791456 for details.
+                         * The colon version was handled above separately.
+                         * This branch here is reached when the separators are semicolons. */
 			if ((i + 1) < n_params) {
-				int32_t color;
-
                                 ++i;
-                                if (params.has_number_at_unchecked(i)) {
-					/* Only semicolons as separators. */
-                                        color = parse_sgr_38_48_parameters(params, &i);
-                                } else if (params.has_subparams_at_unchecked(i)) {
-					/* The first separator was a semicolon, the rest are colons. */
-                                        auto subparams = params.subparams_at_unchecked(i);
-
-					unsigned int index = 0;
-                                        color = parse_sgr_38_48_parameters(subparams, &index);
-					/* Bail out on additional colon-separated values. */
-                                        if (G_UNLIKELY(index != subparams.size() - 1))
-						break;
-				} else {
-					break;
-				}
+                                auto color = parse_sgr_38_48_parameters(params, &i, false);
 				if (G_LIKELY (color != -1)) {
 					if (param == 38) {
                                                 m_defaults.attr.fore = color;