[PATCH v2 2/3] Fix the ':A' word modifier on paths with '..' components.

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

The fix is to stop calling chabspath() at the top of chrealpath().

Preserve the old behaviour when CHASE_DOTS is set.

Also remove an incorrect comment (passing a non-absolute path would have been
fine because the chabspath() call would have made it absolute).
---
 Doc/Zsh/expn.yo    | 12 +++++++++---
 Doc/Zsh/options.yo |  7 ++++++-
 README             |  9 +++++++++
 Src/hist.c         | 27 +++++++++++++++------------
 Test/D02glob.ztst  | 11 ++++++++++-
 5 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index c6e7b6f..82d2c9f 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -225,9 +225,15 @@ intervening directories do not exist.
 )
 item(tt(A))(
 As `tt(a)', but also resolve use of symbolic links where possible.
-Note that resolution of `tt(..)' occurs em(before) resolution of symbolic
-links.  This call is equivalent to tt(a) unless your system has the
-tt(realpath) system call (modern systems do).
+
+By default, symbolic links are resolved before `tt(..)' components.  However,
+if the tt(CHASE_DOTS) option is set, `tt(..)' components are resolved before
+symbolic links.  Furthermore, on systems that lack the tt(realpath+LPAR()RPAR())
+system call (modern systems have it), symbolic links are never resolved and
+this modifier is equivalent to the `tt(a)' modifier.
+
+em(Note): In zsh 5.2 and earlier, resolution of `tt(..)' occurred em(before)
+resolution of symbolic links regardless of the tt(CHASE_DOTS) option.
 )
 item(tt(c))(
 Resolve a command name into an absolute path by searching the command
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 0eaaeca..da93912 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -106,6 +106,11 @@ Without this option set, `tt(cd /foo/bar/..)' changes to tt(/foo); with it
 set, it changes to tt(/alt).  The same applies if the current directory
 is tt(/foo/bar) and `tt(cd ..)' is used.  Note that all other symbolic
 links in the path will also be resolved.
+
+This option also affects the interpretation of the `tt(:A)' history modifier,
+see
+ifzman(the section `Modifiers' in zmanref(zshexpn))\
+ifnzman(noderef(Modifiers)).
 )
 pindex(CHASE_LINKS)
 pindex(NO_CHASE_LINKS)
@@ -569,7 +574,7 @@ Substitutions using the tt(:s) and tt(:&) history modifiers are performed
 with pattern matching instead of string matching.  This occurs wherever
 history modifiers are valid, including glob qualifiers and parameters.
 See
-ifzman(the section Modifiers in zmanref(zshexpn))\
+ifzman(the section `Modifiers' in zmanref(zshexpn))\
 ifnzman(noderef(Modifiers)).
 )
 pindex(IGNORE_BRACES)
diff --git a/README b/README
index d5343db..6857253 100644
--- a/README
+++ b/README
@@ -79,6 +79,15 @@ Other aspects of EXIT trap handling have not changed --- there is still
 only one EXIT trap at any point in a programme, so it is not generally
 useful to combine POSIX and non-POSIX behaviour in the same script.
 
+4) On systems that have the realpath(3) library function, the ':A' word
+modifier now resolves symbolic links before '..' path components.  This
+could lead to different, but usually more desirable, results: the
+tranformed value will now always identify the same directory entry as the
+the pre-transformation value.
+
+The behaviour of 5.2 and older can be achieved by chaining modifiers,
+as in '<expression>:a:A', or by setting the CHASE_DOTS option.
+
 Incompatibilities between 5.0.8 and 5.2
 ---------------------------------------
 
diff --git a/Src/hist.c b/Src/hist.c
index 5fc40bd..30a1bef 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1837,8 +1837,8 @@ chabspath(char **junkptr)
 int
 chrealpath(char **junkptr)
 {
-    char *str;
 #ifdef HAVE_REALPATH
+    char *str;
 # ifdef REALPATH_ACCEPTS_NULL
     char *lastpos, *nonreal, *real;
 # else
@@ -1850,19 +1850,22 @@ chrealpath(char **junkptr)
     if (!**junkptr)
 	return 1;
 
-    /* Notice that this means ..'s are applied before symlinks are resolved! */
-    if (!chabspath(junkptr))
-	return 0;
-
 #ifndef HAVE_REALPATH
-    return 1;
+    return chabspath(junkptr);
 #else
-    /*
-     * Notice that this means you cannot pass relative paths into this
-     * function!
-     */
-    if (**junkptr != '/')
+
+    /* With CHASE_DOTS, resolve '..' components before symlinks.  (This was always
+     * done first in 5.2 and earlier.) */
+    if (isset(CHASEDOTS))
+      chabspath(junkptr);
+
+    if (**junkptr != '/') {
+	*junkptr = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", *junkptr);
+    }
+    if (**junkptr != '/') {
+	/* Can happen after 'rmdir $PWD; zsh' */
 	return 0;
+    }
 
     unmetafy(*junkptr, NULL);
 
@@ -1909,9 +1912,9 @@ chrealpath(char **junkptr)
     } else {
 	*junkptr = metafy(nonreal, lastpos - nonreal + 1, META_HEAPDUP);
     }
-#endif
 
     return 1;
+#endif
 }
 
 /**/
diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
index 7befbc2..bec9826 100644
--- a/Test/D02glob.ztst
+++ b/Test/D02glob.ztst
@@ -673,8 +673,17 @@
 
  ln -s dir3/subdir glob.tmp/link
  () {
+   setopt localoptions chasedots
    print ${1:A} | grep glob.tmp
  } glob.tmp/link/../../hello
  rm glob.tmp/link
-0:modifier ':A' resolves '..' components before symlinks
+0:modifier ':A' resolves '..' components before symlinks with CHASE_DOTS
 # There should be no output
+
+ ln -s dir3/subdir glob.tmp/link
+ () {
+   print ${1:A} | grep glob.tmp
+ } glob.tmp/link/../../hello
+ rm glob.tmp/link
+0:modifier ':A' resolves symlinks before '..' components with NO_CHASE_DOTS
+*>*glob.tmp/hello