From 81a987c39cb414b6b484621c92424b2ddbe36ded Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Thu, 26 Apr 2018 22:42:48 +0200
Subject: [PATCH] Fix range expansion with negative ends

If just one of the range ends is negative, this now forces direction away from it.

I.e. if the beginning is negative, we go in reverse.
If the end is negative, we go forwards.

This fixes cases like

    $var[2..-1]

if $var only has one element.
---
 doc_src/index.hdr.in | 7 +++++++
 src/expand.cpp       | 6 ++++++
 tests/expansion.in   | 3 ++-
 tests/expansion.out  | 5 +++--
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in
index 29555861c..3ea257286 100644
--- a/doc_src/index.hdr.in
+++ b/doc_src/index.hdr.in
@@ -631,6 +631,8 @@ Examples:
 
 Both command substitution and shell variable expansion support accessing only specific items by providing a set of indices in square brackets. It's often needed to access a sequence of elements. To do this, use the range operator '`..`' for this. A range '`a..b`', where range limits 'a' and 'b' are integer numbers, is expanded into a sequence of indices '`a a+1 a+2 ... b`' or '`a a-1 a-2 ... b`' depending on which of 'a' or 'b' is higher. The negative range limits are calculated from the end of the array or command substitution. Note that invalid indexes for either end are silently clamped to one or the size of the array as appropriate.
 
+Range expansion will go in reverse if the end element is earlier in the list than the start, unless exactly one of the given indices is negative. This is to enable clamping without changing direction if the list has fewer elements than expected.
+
 Some examples:
 
 \fish
@@ -649,6 +651,11 @@ echo (seq 10)[-1..1]
 # Uses elements from the last output line to
 # the first one in reverse direction
 # Output is: 10 9 8 7 6 5 4 3 2 1
+
+# The command substitution has only one line,
+# so these will result in empty output:
+echo (echo one)[2..-1]
+echo (echo one)[-3..1]
 \endfish
 
 The same works when setting or expanding variables:
diff --git a/src/expand.cpp b/src/expand.cpp
index 6eb0aec10..16edc703b 100644
--- a/src/expand.cpp
+++ b/src/expand.cpp
@@ -228,6 +228,12 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long
             i2 = i2 < size ? i2 : size;
             // debug( 0, L"Push range idx %d %d", i1, i2 );
             short direction = i2 < i1 ? -1 : 1;
+            // If only the beginning is negative, always go reverse.
+            // If only the end, always go forward.
+            // Prevents `[x..-1]` from going reverse if less than x elements are there.
+            if (tmp1 > -1 != tmp > -1) {
+                direction = tmp1 > -1 ? -1 : 1;
+            }
             for (long jjj = i1; jjj * direction <= i2 * direction; jjj += direction) {
                 // debug(0, L"Expand range [subst]: %i\n", jjj);
                 idx.push_back(jjj);
diff --git a/tests/expansion.in b/tests/expansion.in
index 6a3832baf..2510ac1cf 100644
--- a/tests/expansion.in
+++ b/tests/expansion.in
@@ -65,7 +65,7 @@ expansion "$$foo"
 expansion $$foo
 expansion "prefix$$foo"
 expansion prefix$$foo
-expansion $foo[-5..2]
+expansion $foo[-5..2] # No result, because the starting index is invalid and we force-reverse.
 expansion $foo[-2..-1]
 expansion $foo[-10..-5]
 expansion (printf '%s\n' $foo)[-5..2]
@@ -87,6 +87,7 @@ set -l foo a b c
 expansion $foo[17]
 expansion $foo[-17]
 expansion $foo[17..18]
+expansion $foo[4..-2]
 
 echo "$foo[d]"
 echo $foo[d]
diff --git a/tests/expansion.out b/tests/expansion.out
index 008a89552..6b8ab0743 100644
--- a/tests/expansion.out
+++ b/tests/expansion.out
@@ -36,10 +36,10 @@
 2 baz quux
 1 prefixbaz quux  fooest
 2 prefixbaz prefixquux
-2 bar 
+0
 2  fooest
 0
-2 bar 
+0
 2  fooest
 0
 1 
@@ -55,6 +55,7 @@
 0
 0
 0
+0
 
 
 ####################