Have load_config() and write_(min_)config() return messages

Hardcoding load_config() and write_(min_)config() to write any message
to stdout is awkward, because it means that the message can't be easily
reused when stdout is the wrong place to write it to (e.g. in
menuconfig/guiconfig). This gets extra bad now that there's also the "No
change to ..." message.

Modify load_config() and write_(min_)config() to return the message as a
string instead, and have them always return a message, instead of just
when 'filename' is None and verbose=True. This makes things flexible and
straightforward.

Use the new behavior in menuconfig.py and guiconfig.py. They now show
"No change to ..." when saving a file doesn't modify it.

Tools that want to write messages to stdout should now do
print(kconf.load_config()) / print(kconf.write_config()).

There's no clean way to preserve perfect backwards compatibility here,
but keep accepting the 'verbose' argument and print a deprecation
warning if a value is ever passed for it. That way, scripts will keep
running, though possibly with less output on stdout.

This changes the meaning of the load_config() return value as well,
though I suspect it was only ever used by the menuconfig/guiconfig
interfaces.

The new behavior applies for kconfiglib.VERSION >= (12, 0, 0).

1 files changed, 143 insertions, 101 deletions

diff --git a/kconfiglib.py b/kconfiglib.py
index 01ba0b8..d33a690 100644
--- a/kconfiglib.py
+++ b/kconfiglib.py
@@ -1058,7 +1058,7 @@ class Kconfig(object):
 
         return None
 
-    def load_config(self, filename=None, replace=True, verbose=True):
+    def load_config(self, filename=None, replace=True, verbose=None):
         """
         Loads symbol values from a file in the .config format. Equivalent to
         calling Symbol.set_value() to set each of the values.
@@ -1106,36 +1106,40 @@ class Kconfig(object):
           If True, all existing user values will be cleared before loading the
           .config. Pass False to merge configurations.
 
-        verbose (default: True):
-          If True and filename is None (automatically infer configuration
-          file), a message will be printed to stdout telling which file got
-          loaded (or that no file got loaded). This is meant to reduce
-          boilerplate in tools.
+        verbose (default: None):
+          Limited backwards compatibility to prevent crashes. A warning is
+          printed if anything but None is passed.
 
-        Returns True if an existing configuration was loaded (that didn't come
-        from the 'option defconfig_list' symbol), and False otherwise. This is
-        mostly useful in conjunction with filename=None, as True will always be
-        returned otherwise.
+          Prior to Kconfiglib 12.0.0, this option enabled printing of messages
+          to stdout when 'filename' was None. A message is (always) returned
+          now instead, which is more flexible.
+
+          Will probably be removed in some future version.
+
+        Returns a string with a message saying which file got loaded (or
+        possibly that no file got loaded, when 'filename' is None). This is
+        meant to reduce boilerplate in tools, which can do e.g.
+        print(kconf.load_config()).
         """
-        loaded_existing = True
+        if verbose is not None:
+            _warn_verbose_deprecated("load_config")
+
+        msg = None
         if filename is None:
             filename = standard_config_filename()
-            if exists(filename):
-                if verbose:
-                    print("Using existing configuration '{}' as base"
-                          .format(filename))
-            else:
-                filename = self.defconfig_filename
-                if filename is None:
-                    if verbose:
-                        print("Using default symbol values as base")
-                    return False
+            if not exists(filename) and \
+               not exists(join(self.srctree, filename)):
+                defconfig = self.defconfig_filename
+                if defconfig is None:
+                    return "Using default symbol values (no '{}')" \
+                           .format(filename)
 
-                if verbose:
-                    print("Using default configuration found in '{}' as "
-                          "base".format(filename))
+                msg = " default configuration '{}' (no '{}')" \
+                      .format(defconfig, filename)
+                filename = defconfig
 
-                loaded_existing = False
+        if not msg:
+            msg = " configuration '{}'".format(filename)
 
         # Disable the warning about assigning to symbols without prompts. This
         # is normal and expected within a .config file.
@@ -1149,7 +1153,7 @@ class Kconfig(object):
         finally:
             self._warn_for_no_prompt = True
 
-        return loaded_existing
+        return ("Loaded" if replace else "Merged") + msg
 
     def _load_config(self, filename, replace):
         with self._open_config(filename) as f:
@@ -1367,7 +1371,7 @@ class Kconfig(object):
 
     def write_config(self, filename=None,
                      header="# Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)\n",
-                     save_old=True, verbose=True):
+                     save_old=True, verbose=None):
         r"""
         Writes out symbol values in the .config format. The format matches the
         C implementation, including ordering.
@@ -1406,21 +1410,29 @@ class Kconfig(object):
           due to being a directory, or <filename> being something like
           /dev/null).
 
-        verbose (default: True):
-          If True and filename is None (automatically infer configuration
-          file), a message will be printed to stdout telling which file got
-          written. This is meant to reduce boilerplate in tools.
+        verbose (default: None):
+          Limited backwards compatibility to prevent crashes. A warning is
+          printed if anything but None is passed.
+
+          Prior to Kconfiglib 12.0.0, this option enabled printing of messages
+          to stdout when 'filename' was None. A message is (always) returned
+          now instead, which is more flexible.
+
+          Will probably be removed in some future version.
+
+        Returns a string with a message saying which file got saved. This is
+        meant to reduce boilerplate in tools, which can do e.g.
+        print(kconf.write_config()).
         """
+        if verbose is not None:
+            _warn_verbose_deprecated("write_config")
+
         if filename is None:
             filename = standard_config_filename()
-        else:
-            verbose = False
 
         contents = self._config_contents(header)
         if self._contents_eq(filename, contents):
-            if verbose:
-                 print("No change to '{}'".format(filename))
-            return
+            return "No change to '{}'".format(filename)
 
         if save_old:
             _save_old(filename)
@@ -1428,8 +1440,7 @@ class Kconfig(object):
         with self._open(filename, "w") as f:
             f.write(contents)
 
-        if verbose:
-            print("Configuration written to '{}'".format(filename))
+        return "Configuration saved to '{}'".format(filename)
 
     def _config_contents(self, header):
         # write_config() helper. Returns the contents to write as a string,
@@ -1524,34 +1535,53 @@ class Kconfig(object):
           Text that will be inserted verbatim at the beginning of the file. You
           would usually want each line to start with '#' to make it a comment,
           and include a final terminating newline.
+
+        Returns a string with a message saying which file got saved. This is
+        meant to reduce boilerplate in tools, which can do e.g.
+        print(kconf.write_min_config()).
         """
+        contents = self._min_config_contents(header)
+        if self._contents_eq(filename, contents):
+            return "No change to '{}'".format(filename)
+
         with self._open(filename, "w") as f:
-            f.write(header)
+            f.write(contents)
 
-            for sym in self.unique_defined_syms:
-                # Skip symbols that cannot be changed. Only check
-                # non-choice symbols, as selects don't affect choice
-                # symbols.
-                if not sym.choice and \
-                   sym.visibility <= expr_value(sym.rev_dep):
-                    continue
+        return "Minimal configuration saved to '{}'".format(filename)
 
-                # Skip symbols whose value matches their default
-                if sym.str_value == sym._str_default():
-                    continue
+    def _min_config_contents(self, header):
+        # write_min_config() helper. Returns the contents to write as a string,
+        # with 'header' at the beginning.
 
-                # Skip symbols that would be selected by default in a
-                # choice, unless the choice is optional or the symbol type
-                # isn't bool (it might be possible to set the choice mode
-                # to n or the symbol to m in those cases).
-                if sym.choice and \
-                   not sym.choice.is_optional and \
-                   sym.choice._get_selection_from_defaults() is sym and \
-                   sym.orig_type is BOOL and \
-                   sym.tri_value == 2:
-                    continue
+        chunks = [header]
+        add = chunks.append
 
-                f.write(sym.config_string)
+        for sym in self.unique_defined_syms:
+            # Skip symbols that cannot be changed. Only check
+            # non-choice symbols, as selects don't affect choice
+            # symbols.
+            if not sym.choice and \
+               sym.visibility <= expr_value(sym.rev_dep):
+                continue
+
+            # Skip symbols whose value matches their default
+            if sym.str_value == sym._str_default():
+                continue
+
+            # Skip symbols that would be selected by default in a
+            # choice, unless the choice is optional or the symbol type
+            # isn't bool (it might be possible to set the choice mode
+            # to n or the symbol to m in those cases).
+            if sym.choice and \
+               not sym.choice.is_optional and \
+               sym.choice._get_selection_from_defaults() is sym and \
+               sym.orig_type is BOOL and \
+               sym.tri_value == 2:
+                continue
+
+            add(sym.config_string)
+
+        return "".join(chunks)
 
     def sync_deps(self, path):
         """
@@ -2083,7 +2113,7 @@ class Kconfig(object):
         # filesystem, compare the files, and rename() the temporary file if it
         # differs, but it breaks stuff like write_config("/dev/null"), which is
         # used out there to force evaluation-related warnings to be generated.
-        # This simple version pretty failsafe and portable.
+        # This simple version is pretty failsafe and portable.
 
         if not self._contents_eq(filename, contents):
             with self._open(filename, "w") as f:
@@ -5989,6 +6019,10 @@ def load_allconfig(kconf, filename):
       Command-specific configuration filename - "allyes.config",
       "allno.config", etc.
     """
+    allconfig = os.environ.get("KCONFIG_ALLCONFIG")
+    if allconfig is None:
+        return
+
     def std_msg(e):
         # "Upcasts" a _KconfigIOError to an IOError, removing the custom
         # __str__() message. The standard message is better here.
@@ -5997,25 +6031,23 @@ def load_allconfig(kconf, filename):
     kconf.disable_override_warnings()
     kconf.disable_redun_warnings()
 
-    allconfig = os.environ.get("KCONFIG_ALLCONFIG")
-    if allconfig is not None:
-        if allconfig in ("", "1"):
-            try:
-                kconf.load_config(filename, False)
-            except IOError as e1:
-                try:
-                    kconf.load_config("all.config", False)
-                except IOError as e2:
-                    sys.exit("error: KCONFIG_ALLCONFIG is set, but neither {} "
-                             "nor all.config could be opened: {}, {}"
-                             .format(filename, std_msg(e1), std_msg(e2)))
-        else:
+    if allconfig in ("", "1"):
+        try:
+            print(kconf.load_config(filename, False))
+        except IOError as e1:
             try:
-                kconf.load_config(allconfig, False)
-            except IOError as e:
-                sys.exit("error: KCONFIG_ALLCONFIG is set to '{}', which "
-                         "could not be opened: {}"
-                         .format(allconfig, std_msg(e)))
+                print(kconf.load_config("all.config", False))
+            except IOError as e2:
+                sys.exit("error: KCONFIG_ALLCONFIG is set, but neither {} "
+                         "nor all.config could be opened: {}, {}"
+                         .format(filename, std_msg(e1), std_msg(e2)))
+    else:
+        try:
+            print(kconf.load_config(allconfig, False))
+        except IOError as e:
+            sys.exit("error: KCONFIG_ALLCONFIG is set to '{}', which "
+                     "could not be opened: {}"
+                     .format(allconfig, std_msg(e)))
 
     # API wart: It would be nice if there was a way to query and/or push/pop
     # warning settings
@@ -6166,28 +6198,6 @@ def _save_old(path):
         pass
 
 
-def _decoding_error(e, filename, macro_linenr=None):
-    # Gives the filename and context for UnicodeDecodeError's, which are a pain
-    # to debug otherwise. 'e' is the UnicodeDecodeError object.
-    #
-    # If the decoding error is for the output of a $(shell,...) command,
-    # macro_linenr holds the line number where it was run (the exact line
-    # number isn't available for decoding errors in files).
-
-    raise KconfigError(
-        "\n"
-        "Malformed {} in {}\n"
-        "Context: {}\n"
-        "Problematic data: {}\n"
-        "Reason: {}".format(
-            e.encoding,
-            "'{}'".format(filename) if macro_linenr is None else
-                "output from macro at {}:{}".format(filename, macro_linenr),
-            e.object[max(e.start - 40, 0):e.end + 40],
-            e.object[e.start:e.end],
-            e.reason))
-
-
 def _name_and_loc(sc):
     # Helper for giving the symbol/choice name and location(s) in e.g. warnings
 
@@ -6485,6 +6495,38 @@ def _found_dep_loop(loop, cur):
     raise KconfigError(msg)
 
 
+def _decoding_error(e, filename, macro_linenr=None):
+    # Gives the filename and context for UnicodeDecodeError's, which are a pain
+    # to debug otherwise. 'e' is the UnicodeDecodeError object.
+    #
+    # If the decoding error is for the output of a $(shell,...) command,
+    # macro_linenr holds the line number where it was run (the exact line
+    # number isn't available for decoding errors in files).
+
+    raise KconfigError(
+        "\n"
+        "Malformed {} in {}\n"
+        "Context: {}\n"
+        "Problematic data: {}\n"
+        "Reason: {}".format(
+            e.encoding,
+            "'{}'".format(filename) if macro_linenr is None else
+                "output from macro at {}:{}".format(filename, macro_linenr),
+            e.object[max(e.start - 40, 0):e.end + 40],
+            e.object[e.start:e.end],
+            e.reason))
+
+
+def _warn_verbose_deprecated(fn_name):
+    sys.stderr.write(
+        "Deprecation warning: {0}()'s 'verbose' argument has no effect. Since "
+        "Kconfiglib 12.0.0, the message is returned from {0}() instead, "
+        "and is always generated. Do e.g. print(kconf.{0}()) if you want to "
+        "want to show a message like \"Loaded configuration '.config'\" on "
+        "stdout. The old API required ugly hacks to reuse messages in "
+        "configuration interfaces.\n".format(fn_name))
+
+
 # Predefined preprocessor functions