From 9ef47a43a4127c16df468ee24f488358073dfa2c Mon Sep 17 00:00:00 2001
From: Kurtis Rader <krader@skepticism.us>
Date: Thu, 20 Jul 2017 17:54:06 -0700
Subject: [PATCH] change how `argparse` handles boolean flags

When reporting whether a boolean flag was seen report the actual flags
rather than a summary count. For example, if you have option spec `h/help`
and we parse `-h --help -h` don't do the equivalent of `set _flag_h 3`
do `set _flag_h -h --help -h`.

Partial fix for #4226
---
 doc_src/argparse.txt     |  2 +-
 src/builtin_argparse.cpp |  8 ++++++++
 tests/argparse.out       | 16 ++++++++--------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/doc_src/argparse.txt b/doc_src/argparse.txt
index ae763382d..fc95946e0 100644
--- a/doc_src/argparse.txt
+++ b/doc_src/argparse.txt
@@ -13,7 +13,7 @@ Each OPTION_SPEC can be written in the domain specific language <a href="#argpar
 
 Each option that is seen in the ARG list will result in a var name of the form `_flag_X`, where `X` is the short flag letter and the long flag name. The OPTION_SPEC always requires a short flag even if it can't be used. So there will always be `_flag_X` var set using the short flag letter if the corresponding short or long flag is seen. The long flag name var (e.g., `_flag_help`) will only be defined, obviously, if the OPTION_SPEC includes a long flag name.
 
-For example `_flag_h` and `_flag_help` if `-h` or `--help` is seen. The var will be set with local scope (i.e., as if the script had done `set -l _flag_X`). If the flag is a boolean (that is, does not have an associated value) the value is a count of how many times the flag was seen. If the option can have zero or more values the flag var will have zero or more values corresponding to the values collected when the ARG list is processed. If the flag was not seen the flag var will not be set.
+For example `_flag_h` and `_flag_help` if `-h` or `--help` is seen. The var will be set with local scope (i.e., as if the script had done `set -l _flag_X`). If the flag is a boolean (that is, does not have an associated value) the values are the short and long flags seen. If the option is not a boolean flag the values will be zero or more values corresponding to the values collected when the ARG list is processed. If the flag was not seen the flag var will not be set.
 
 The following `argparse` options are available. They must appear before all OPTION_SPECs:
 
diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp
index 4488317a4..6d6518976 100644
--- a/src/builtin_argparse.cpp
+++ b/src/builtin_argparse.cpp
@@ -437,6 +437,7 @@ static void populate_option_strings(
 // Add a count for how many times we saw each boolean flag but only if we saw the flag at least
 // once.
 static void update_bool_flag_counts(argparse_cmd_opts_t &opts) {
+    return;
     for (auto it : opts.options) {
         auto opt_spec = it.second;
         // The '#' short flag is special. It doesn't take any values but isn't a boolean arg.
@@ -539,6 +540,13 @@ static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_
         option_spec_t *opt_spec = found->second;
         opt_spec->num_seen++;
         if (opt_spec->num_allowed == 0) {
+            // It's a boolean flag. Save the flag we saw since it might be useful to know if the
+            // short or long flag was given.
+            if (long_idx == -1) {
+                opt_spec->vals.push_back(wcstring(1, L'-') + opt_spec->short_flag);
+            } else {
+                opt_spec->vals.push_back(L"--" + opt_spec->long_flag);
+            }
             assert(!w.woptarg);
             long_idx = -1;
             continue;
diff --git a/tests/argparse.out b/tests/argparse.out
index 71df1f721..eb3752ec8 100644
--- a/tests/argparse.out
+++ b/tests/argparse.out
@@ -2,8 +2,8 @@
 # One arg and no matching flags
 argv help
 # Five args with two matching a flag
-_flag_h 2
-_flag_help 2
+_flag_h '--help'  '-h'
+_flag_help '--help'  '-h'
 argv 'help'  'me'  'a lot more'
 # Required, optional, and multiple flags
 _flag_a ABC
@@ -12,23 +12,23 @@ _flag_d
 _flag_def
 _flag_g 'g1'  'g2'  'g3'
 _flag_ghk 'g1'  'g2'  'g3'
-_flag_h 1
-_flag_help 1
+_flag_h --help
+_flag_help --help
 argv 'help'  'me'
 # --stop-nonopt works
 _flag_a A2
 _flag_abc A2
-_flag_h 1
-_flag_help 1
+_flag_h -h
+_flag_help -h
 argv 'non-opt'  'second non-opt'  '--help'
 # Implicit int flags work
 _flag_val 123
 argv 'abc'  'def'
 _flag_t woohoo
 _flag_token woohoo
-_flag_v 2
+_flag_v '-v'  '--verbose'
 _flag_val -234
-_flag_verbose 2
+_flag_verbose '-v'  '--verbose'
 argv 'a1'  'a2'
 # Should be set to 987
 _flag_m 987