From 4221d6c3e677bf4562192b6de91a0f2d919c7093 Mon Sep 17 00:00:00 2001
From: Aaron Gyes <me@aaron.gy>
Date: Sun, 18 Nov 2018 12:23:36 -0800
Subject: [PATCH] Realpath styling tweaks.

Add braces I forgot, improve comments, make line spacing more
consistent around if blocks.
---
 src/builtin_realpath.cpp | 19 +++++++++----------
 src/wutil.cpp            |  8 ++++----
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/src/builtin_realpath.cpp b/src/builtin_realpath.cpp
index f7a0fd1e1..b87fe905a 100644
--- a/src/builtin_realpath.cpp
+++ b/src/builtin_realpath.cpp
@@ -12,18 +12,16 @@
 #include "io.h"
 #include "wutil.h"  // IWYU pragma: keep
 
-/// An implementation of the external realpath command that doesn't support any options. It's meant
-/// to be used only by scripts which need to be portable. In general scripts shouldn't invoke this
-/// directly. They should just use `realpath` which will fallback to this builtin if an external
-/// command cannot be found. This behaves like the external `realpath --canonicalize-existing`;
-/// that is, it requires all path components, including the final, to exist.
+/// An implementation of the external realpath command. Desn't support any options.
+/// In general scripts shouldn't invoke this directly. They should just use `realpath` which
+/// will fallback to this builtin if an external command cannot be found.
 int builtin_realpath(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
     help_only_cmd_opts_t opts;
-
+    int argc = builtin_count_args(argv);
     int optind;
     int retval = parse_help_only_cmd_opts(opts, &optind, argc, argv, parser, streams);
+
     if (retval != STATUS_CMD_OK) return retval;
 
     if (opts.print_help) {
@@ -31,7 +29,7 @@ int builtin_realpath(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         return STATUS_CMD_OK;
     }
 
-    if (optind + 1 != argc) {
+    if (optind + 1 != argc) { // TODO: allow arbitrary args. `realpath *` should print many paths
         streams.err.append_format(BUILTIN_ERR_ARG_COUNT1, cmd, 1, argc - optind);
         builtin_print_help(parser, streams, cmd, streams.out);
         return STATUS_INVALID_ARGS;
@@ -40,14 +38,15 @@ int builtin_realpath(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     if (auto real_path = wrealpath(argv[optind])) {
         streams.out.append(*real_path);
     } else {
-        if (errno)
+        if (errno) {
             // realpath() just couldn't do it. Report the error and make it clear
             // this is an error from our builtin, not the system's realpath.
             streams.err.append_format(L"builtin %ls: %ls: %s\n", cmd, argv[optind],
                                       strerror(errno));
-        else
+        } else {
             // Who knows. Probably a bug in our wrealpath() implementation.
             streams.err.append_format(_(L"builtin %ls: Invalid path: %ls\n"), cmd, argv[optind]);
+        }
 
         return STATUS_CMD_ERROR;
     }
diff --git a/src/wutil.cpp b/src/wutil.cpp
index 8287b5bb6..0dc416f40 100644
--- a/src/wutil.cpp
+++ b/src/wutil.cpp
@@ -388,8 +388,7 @@ maybe_t<wcstring> wrealpath(const wcstring &pathname) {
     cstring real_path;
     cstring narrow_path = wcs2string(pathname);
 
-    // Strip trailing slashes. This is needed to be bug-for-bug compatible with GNU realpath which
-    // treats "/a//" as equivalent to "/a" whether or not /a exists.
+    // Strip trailing slashes. This is treats "/a//" as equivalent to "/a" if /a is a non-directory.
     while (narrow_path.size() > 1 && narrow_path.at(narrow_path.size() - 1) == '/') {
         narrow_path.erase(narrow_path.size() - 1, 1);
     }
@@ -410,6 +409,7 @@ maybe_t<wcstring> wrealpath(const wcstring &pathname) {
         } else {
             // Only call realpath() on the portion up to the last component.
             errno = 0;
+
             if (pathsep_idx == cstring::npos) {
                 // If there is no "/", this is a file in $PWD, so give the realpath to that.
                 narrow_res = realpath(".", tmpbuf);
@@ -422,11 +422,11 @@ maybe_t<wcstring> wrealpath(const wcstring &pathname) {
             if (!narrow_res) return none();
 
             pathsep_idx++;
-
             real_path.append(narrow_res);
 
-            // This test is to deal with pathological cases such as /../../x => //x.
+            // This test is to deal with cases such as /../../x => //x.
             if (real_path.size() > 1) real_path.append("/");
+
             real_path.append(narrow_path.substr(pathsep_idx, cstring::npos));
         }
     }