oldconfig: Use Kconfig.node_iter() and clean up

Some general cleanup:

 - Handle the iteration with the new Kconfig.node_iter() helper.

   This makes some function so short that they become pointless. Have
   just main() + oldconfig(node) + small helper functions.

 - Use _name_and_loc_str(sc) for choices too, so that all locations get
   reported for named choices defined in multiple locations

 - Rewrite the intro to be less wordy and remove the sample session
   (leftover from when oldconfig.py was in examples/, and not that
   exciting)

 - Print "Updated configuration written to..." instead of
   "Configuration saved to..."

 - Various other nits

3 files changed, 71 insertions, 169 deletions

diff --git a/menuconfig.py b/menuconfig.py
index ee79864..25047f3 100755
--- a/menuconfig.py
+++ b/menuconfig.py
@@ -268,10 +268,7 @@ def _expr_str(expr):
     # Custom expression printer that shows symbol values
     return expr_str(expr, _name_and_val_str)
 
-
-# Entry point when run as an executable, split out so that setuptools'
-# 'entry_points' can be used. It produces a handy menuconfig.exe launcher on
-# Windows.
+# Note: Used as the entry point in setup.py
 def _main():
     menuconfig(standard_kconfig())
 
diff --git a/oldconfig.py b/oldconfig.py
index 2c058bd..29ca533 100755
--- a/oldconfig.py
+++ b/oldconfig.py
@@ -3,105 +3,21 @@
 # Copyright (c) 2018, Ulf Magnusson
 # SPDX-License-Identifier: ISC
 
-# Implements oldconfig-like functionality:
+# Implements oldconfig functionality:
 #
 #   1. Load existing .config
-#   2. Prompt the user for the value of all changeable symbols/choices
-#      that aren't already set in the .config
+#   2. Prompt the user for the value of all modifiable symbols/choices that
+#      aren't already set in the .config
 #   3. Write new .config
 #
-# Unlike 'make oldconfig', this script doesn't print menu titles and
-# comments, but instead gives the Kconfig locations of all symbols and
-# choices. Printing menu titles and comments as well would be pretty easy to
-# add (look at the parents of each item and print all menu prompts and
-# comments unless they have already been printed).
+# Unlike 'make oldconfig', this script doesn't print menu titles and comments,
+# but gives Kconfig definition locations. Printing menus and comments would be
+# pretty easy to add: Look at the parents of each item and print all menu
+# prompts and comments unless they have already been printed (assuming you want
+# to skip "irrelevant" menus).
 #
-# Inputting '?' on the prompt will display the help text of the item, if any.
-# Hopefully no one will want to use that as a value.
-#
-# Sample session:
-#
-#   OldconfigExample contents:
-#
-#     config MODULES
-#             def_bool y
-#             option modules
-#
-#     config BOOL_SYM
-#             bool "BOOL_SYM prompt"
-#             default y
-#
-#     config TRISTATE_SYM
-#             tristate "TRISTATE_SYM prompt"
-#             default m
-#
-#     config STRING_SYM
-#             string "STRING_SYM prompt"
-#             default "foo"
-#
-#     config INT_SYM
-#             int "INT_SYM prompt"
-#
-#     config HEX_SYM
-#             hex "HEX_SYM prompt"
-#
-#     choice
-#             bool "A choice that defaults to CHOICE_B"
-#             default CHOICE_B
-#
-#     config CHOICE_A
-#             bool "CHOICE_A's prompt"
-#
-#     config CHOICE_B
-#             bool "CHOICE_B's prompt"
-#
-#     config CHOICE_C
-#             bool "CHOICE_C's prompt"
-#
-#     endchoice
-#
-#
-#   Running:
-#
-#     $ touch .config  # Run with empty .config
-#
-#     $ python(3) oldconfig.py Kconfig
-#     BOOL_SYM prompt (BOOL_SYM, defined at Kconfig:5) [n/Y] foo
-#     Invalid tristate value
-#     BOOL_SYM prompt (BOOL_SYM, defined at Kconfig:5) [n/Y] n
-#     TRISTATE_SYM prompt (TRISTATE_SYM, defined at Kconfig:9) [n/M/y]
-#     STRING_SYM prompt (STRING_SYM, defined at Kconfig:13) [foo] bar
-#     INT_SYM prompt (INT_SYM, defined at Kconfig:17) [] 0x123
-#     warning: the value '0x123' is invalid for INT_SYM (defined at Kconfig:17), which has type int. Assignment ignored.
-#     INT_SYM prompt (INT_SYM, defined at Kconfig:17) [] 123
-#     HEX_SYM prompt (HEX_SYM, defined at Kconfig:20) [] 0x123
-#     A choice that default to B (defined at Kconfig:23)
-#       1. CHOICE_A's prompt (CHOICE_A)
-#     > 2. CHOICE_B's prompt (CHOICE_B)
-#       3. CHOICE_C's prompt (CHOICE_C)
-#     choice[1-3]: 5
-#     Bad index
-#     A choice that default to B (defined at Kconfig:23)
-#       1. CHOICE_A's prompt (CHOICE_A)
-#     > 2. CHOICE_B's prompt (CHOICE_B)
-#       3. CHOICE_C's prompt (CHOICE_C)
-#     choice[1-3]: 3
-#     Configuration written to .config
-#
-#     $ cat .config
-#     # Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)
-#     CONFIG_MODULES=y
-#     # CONFIG_BOOL_SYM is not set
-#     CONFIG_TRISTATE_SYM=m
-#     CONFIG_STRING_SYM="bar"
-#     CONFIG_INT_SYM=123
-#     CONFIG_HEX_SYM=0x123
-#     # CONFIG_CHOICE_A is not set
-#     # CONFIG_CHOICE_B is not set
-#     CONFIG_CHOICE_C=y
-#
-#     $ python oldconfig.py Kconfig  # Everything's already up to date
-#     Configuration written to .config
+# Entering '?' displays the help text of the symbol/choice, if any.
+
 from __future__ import print_function
 
 import os
@@ -114,46 +30,44 @@ from kconfiglib import Kconfig, Symbol, Choice, BOOL, TRISTATE, HEX, \
 if sys.version_info[0] < 3:
     input = raw_input
 
-def eprint(*args):
-    print(*args, file=sys.stderr)
 
-def print_help(node):
-    if node.help is not None:
-        print("\n" + node.help)
-    else:
-        print("\nNo help text\n")
+# Note: Used as the entry point in setup.py
+def _main():
+    # Earlier symbols in Kconfig files might depend on later symbols and become
+    # visible if their values change. This flag is set to True if the value of
+    # any symbol changes, in which case we rerun the oldconfig to check for new
+    # visible symbols.
+    global conf_changed
 
-def name_and_loc_str(sym):
-    """
-    Helper for printing the symbol name along with the location(s) in the
-    Kconfig files where the symbol is defined
-    """
-    return "{}, defined at {}".format(
-        sym.name,
-        ", ".join("{}:{}".format(node.filename, node.linenr)
-                  for node in sym.nodes))
+    kconf = standard_kconfig()
 
-def default_value_str(sym):
-    """
-    Returns the "m/M/y" string in e.g.
+    config_filename = standard_config_filename()
+    if not os.path.exists(config_filename):
+        sys.exit("{}: '{}' not found".format(sys.argv[0], config_filename))
 
-      TRISTATE_SYM prompt (TRISTATE_SYM, defined at Kconfig:9) [n/M/y]:
 
-    For string/int/hex, returns the default value as-is.
-    """
-    if sym.type in (BOOL, TRISTATE):
-        return "/".join(("nmy" if sym.tri_value != tri else "NMY")[tri]
-                        for tri in sym.assignable)
+    kconf.load_config(config_filename)
 
-    # string/int/hex
-    return sym.str_value
+    while True:
+        conf_changed = False
+
+        for node in kconf.node_iter():
+            oldconfig(node)
+
+        if not conf_changed:
+            break
+
+    kconf.write_config(config_filename)
 
-def do_oldconfig_for_node(node):
+    print("Updated configuration written to '{}'".format(config_filename))
+
+
+def oldconfig(node):
     """
-    Prompts the user for a value for the menu node item, where applicable in
-    oldconfig mode
+    Prompts the user for a value if node.item is a visible symbol/choice with
+    no user value.
     """
-    # See do_oldconfig()
+    # See main()
     global conf_changed
 
     # Only symbols and choices can be configured
@@ -188,11 +102,11 @@ def do_oldconfig_for_node(node):
         # (for the default value)
         while True:
             val = input("{} ({}) [{}] ".format(
-                            node.prompt[0], name_and_loc_str(sym),
-                            default_value_str(sym)))
+                            node.prompt[0], _name_and_loc_str(sym),
+                            _default_value_str(sym)))
 
             if val == "?":
-                print_help(node)
+                _print_help(node)
                 continue
 
             # Substitute a blank string with the default value the symbol
@@ -248,8 +162,7 @@ def do_oldconfig_for_node(node):
         # Loop until the user enters a valid selection or a blank string (for
         # the default selection)
         while True:
-            print("{} (defined at {}:{})".format(
-                node.prompt[0], node.filename, node.linenr))
+            print("{} ({})".format(node.prompt[0], _name_and_loc_str(choice)))
 
             for i, sym in enumerate(options, 1):
                 print("{} {}. {} ({})".format(
@@ -263,7 +176,7 @@ def do_oldconfig_for_node(node):
             sel_index = input("choice[1-{}]: ".format(len(options)))
 
             if sel_index == "?":
-                print_help(node)
+                _print_help(node)
                 continue
 
             # Pick the default selection if the string is blank
@@ -274,11 +187,11 @@ def do_oldconfig_for_node(node):
             try:
                 sel_index = int(sel_index)
             except ValueError:
-                eprint("Bad index")
+                print("Bad index", file=sys.stderr)
                 continue
 
             if not 1 <= sel_index <= len(options):
-                eprint("Bad index")
+                print("Bad index", file=sys.stderr)
                 continue
 
             # Valid selection
@@ -301,44 +214,36 @@ def do_oldconfig_for_node(node):
             if sym is not choice.user_selection and sym.visibility:
                 sym.set_value(0)
 
-# Entry point when run as an executable, split out so that setuptools'
-# 'entry_points' can be used. It produces a handy oldconfig.exe launcher on
-# Windows.
-def main():
-    kconf = standard_kconfig()
 
-    config_filename = standard_config_filename()
-    if not os.path.exists(config_filename):
-        sys.exit("{}: '{}' does not exist"
-                 .format(sys.argv[0], config_filename))
+def _name_and_loc_str(sc):
+    # Helper for printing the name of the symbol/choice 'sc' along with the
+    # location(s) in the Kconfig files where it is defined. Unnamed choices
+    # return "choice" instead of the name.
 
-    kconf.load_config(config_filename)
-    do_oldconfig(kconf)
-    kconf.write_config(config_filename)
+    return "{}, defined at {}".format(
+        sc.name or "choice",
+        ", ".join("{}:{}".format(node.filename, node.linenr)
+                  for node in sc.nodes))
 
-    print("Configuration saved to '{}'".format(config_filename))
 
-def do_oldconfig(kconf):
-    # An earlier symbol in the Kconfig files might depend on a later symbol and
-    # become visible if its value changes. This flag is set to True if the
-    # value of any symbol changes, in which case we rerun the oldconfig to
-    # check for new visible symbols.
-    global conf_changed
+def _print_help(node):
+    print("\n" + (node.help or "No help text\n"))
 
-    while True:
-        conf_changed = False
-        do_oldconfig_rec(kconf.top_node)
-        if not conf_changed:
-            break
 
-def do_oldconfig_rec(node):
-    while node:
-        do_oldconfig_for_node(node)
+def _default_value_str(sym):
+    # Returns the "m/M/y" string in e.g.
+    #
+    #   TRISTATE_SYM prompt (TRISTATE_SYM, defined at Kconfig:9) [n/M/y]:
+    #
+    # For string/int/hex, returns the default value as-is.
 
-        if node.list:
-            do_oldconfig_rec(node.list)
+    if sym.type in (BOOL, TRISTATE):
+        return "/".join(("NMY" if sym.tri_value == tri else "nmy")[tri]
+                        for tri in sym.assignable)
+
+    # string/int/hex
+    return sym.str_value
 
-        node = node.next
 
 if __name__ == "__main__":
-    main()
+    _main()
diff --git a/setup.py b/setup.py
index 0744e2b..4dc6c9e 100644
--- a/setup.py
+++ b/setup.py
@@ -40,7 +40,7 @@ setuptools.setup(
         "console_scripts": (
             "menuconfig = menuconfig:_main",
             "genconfig = genconfig:main",
-            "oldconfig = oldconfig:main",
+            "oldconfig = oldconfig:_main",
             "alldefconfig = alldefconfig:main",
             "allnoconfig = allnoconfig:main",
             "allmodconfig = allmodconfig:main",