Reverse the logic of --no-XXX option

dlax · dlax · commit fc027febd56e · 2024-05-24T08:39:50.000+02:00
We avoid propagating the negation and instead use a meaningful name as
"dest", set the action to "store_false" and the default to True.

Also the "dest" for "--no-database" is changed to "database" for
consistency with other options.

When building the Flag value, in Flag.load(), we now follow the order of
options definition.

Finally, we define a flag() helper to factor out the common definition
pattern of these options.
diff --git a/pgactivity/cli.py b/pgactivity/cli.py
@@ -7,6 +7,7 @@
 import time
 from argparse import ArgumentParser
 from io import StringIO
+from typing import Any
 
 from blessed import Terminal
 
@@ -44,6 +45,10 @@ def configure_logger(debug_file: str | None = None) -> StringIO:
     return memory_string
 
 
+def flag(p: Any, spec: str, *, dest: str, help: str) -> None:
+    p.add_argument(spec, dest=dest, help=help, action="store_false", default=True)
+
+
 def get_parser() -> ArgumentParser:
     parser = ArgumentParser(
         usage="%(prog)s [options] [connection string]",
@@ -102,29 +107,13 @@ def get_parser() -> ArgumentParser:
         default=None,
     )
     # --no-db-size
-    group.add_argument(
-        "--no-db-size",
-        dest="nodbsize",
-        action="store_true",
-        help="Skip total size of DB.",
-        default=False,
-    )
+    flag(group, "--no-db-size", dest="dbsize", help="Skip total size of DB.")
     # --no-tempfiles
-    group.add_argument(
-        "--no-tempfiles",
-        dest="notempfiles",
-        action="store_true",
-        help="Skip tempfile count and size.",
-        default=False,
+    flag(
+        group, "--no-tempfiles", dest="tempfiles", help="Skip tempfile count and size."
     )
     # --no-walreceiver
-    group.add_argument(
-        "--no-walreceiver",
-        dest="nowalreceiver",
-        action="store_true",
-        help="Skip walreceiver checks.",
-        default=False,
-    )
+    flag(group, "--no-walreceiver", dest="walreceiver", help="Skip walreceiver checks.")
     # --wrap-query
     group.add_argument(
         "-w",
@@ -239,94 +228,29 @@ def get_parser() -> ArgumentParser:
         "Process table display options",
         "These options may be used hide some columns from the processes table.",
     )
+
     # --no-pid
-    group.add_argument(
-        "--no-pid",
-        dest="nopid",
-        action="store_true",
-        help="Disable PID.",
-        default=False,
-    )
+    flag(group, "--no-pid", dest="pid", help="Disable PID.")
     # --no-database
-    group.add_argument(
-        "--no-database",
-        dest="nodb",
-        action="store_true",
-        help="Disable DATABASE.",
-        default=False,
-    )
+    flag(group, "--no-database", dest="database", help="Disable DATABASE.")
     # --no-user
-    group.add_argument(
-        "--no-user",
-        dest="nouser",
-        action="store_true",
-        help="Disable USER.",
-        default=False,
-    )
+    flag(group, "--no-user", dest="user", help="Disable USER.")
     # --no-client
-    group.add_argument(
-        "--no-client",
-        dest="noclient",
-        action="store_true",
-        help="Disable CLIENT.",
-        default=False,
-    )
+    flag(group, "--no-client", dest="client", help="Disable CLIENT.")
     # --no-cpu
-    group.add_argument(
-        "--no-cpu",
-        dest="nocpu",
-        action="store_true",
-        help="Disable CPU%%.",
-        default=False,
-    )
+    flag(group, "--no-cpu", dest="cpu", help="Disable CPU%%.")
     # --no-mem
-    group.add_argument(
-        "--no-mem",
-        dest="nomem",
-        action="store_true",
-        help="Disable MEM%%.",
-        default=False,
-    )
+    flag(group, "--no-mem", dest="mem", help="Disable MEM%%.")
     # --no-read
-    group.add_argument(
-        "--no-read",
-        dest="noread",
-        action="store_true",
-        help="Disable READ/s.",
-        default=False,
-    )
+    flag(group, "--no-read", dest="read", help="Disable READ/s.")
     # --no-write
-    group.add_argument(
-        "--no-write",
-        dest="nowrite",
-        action="store_true",
-        help="Disable WRITE/s.",
-        default=False,
-    )
+    flag(group, "--no-write", dest="write", help="Disable WRITE/s.")
     # --no-time
-    group.add_argument(
-        "--no-time",
-        dest="notime",
-        action="store_true",
-        help="Disable TIME+.",
-        default=False,
-    )
+    flag(group, "--no-time", dest="time", help="Disable TIME+.")
     # --no-wait
-    group.add_argument(
-        "--no-wait",
-        dest="nowait",
-        action="store_true",
-        help="Disable W.",
-        default=False,
-    )
+    flag(group, "--no-wait", dest="wait", help="Disable W.")
     # --no-app-name
-    group.add_argument(
-        "--no-app-name",
-        dest="noappname",
-        action="store_true",
-        help="Disable APP.",
-        default=False,
-    )
+    flag(group, "--no-app-name", dest="appname", help="Disable APP.")
 
     group = parser.add_argument_group("Header display options")
     # --no-inst-info
diff --git a/pgactivity/config.py b/pgactivity/config.py
@@ -110,46 +110,46 @@ def load(
         config: Configuration | None,
         *,
         is_local: bool,
-        noappname: bool,
-        noclient: bool,
-        nocpu: bool,
-        nodb: bool,
-        nomem: bool,
-        nopid: bool,
-        noread: bool,
-        notime: bool,
-        nouser: bool,
-        nowait: bool,
-        nowrite: bool,
+        appname: bool,
+        client: bool,
+        cpu: bool,
+        database: bool,
+        mem: bool,
+        pid: bool,
+        read: bool,
+        time: bool,
+        user: bool,
+        wait: bool,
+        write: bool,
         **kwargs: Any,
     ) -> Flag:
         """Build a Flag value from command line options."""
         if config:
             flag = cls.from_config(config)
         else:
             flag = cls.all()
-        if nodb:
+        if not pid:
+            flag ^= cls.PID
+        if not database:
             flag ^= cls.DATABASE
-        if nouser:
+        if not user:
             flag ^= cls.USER
-        if nocpu:
-            flag ^= cls.CPU
-        if noclient:
+        if not client:
             flag ^= cls.CLIENT
-        if nomem:
+        if not cpu:
+            flag ^= cls.CPU
+        if not mem:
             flag ^= cls.MEM
-        if noread:
+        if not read:
             flag ^= cls.READ
-        if nowrite:
+        if not write:
             flag ^= cls.WRITE
-        if notime:
+        if not time:
             flag ^= cls.TIME
-        if nowait:
+        if not wait:
             flag ^= cls.WAIT
-        if noappname:
+        if not appname:
             flag ^= cls.APPNAME
-        if nopid:
-            flag ^= cls.PID
 
         # Remove some if no running against local pg server.
         if not is_local and (flag & cls.CPU):
diff --git a/pgactivity/ui.py b/pgactivity/ui.py
@@ -29,13 +29,13 @@ def main(
 
     is_local = data.pg_is_local() and data.pg_is_local_access()
 
-    skip_db_size = options.nodbsize
+    skip_db_size = not options.dbsize
     server_information = data.pg_get_server_information(
         prev_server_info=None,
         using_rds=options.rds,
-        skip_db_size=options.nodbsize,
-        skip_tempfile=options.notempfiles,
-        skip_walreceiver=options.nowalreceiver,
+        skip_db_size=skip_db_size,
+        skip_tempfile=not options.tempfiles,
+        skip_walreceiver=not options.walreceiver,
     )
 
     flag = Flag.load(config, is_local=is_local, **vars(options))
@@ -74,7 +74,7 @@ def main(
                 break
             elif not ui.interactive() and key == keys.SPACE:
                 ui.toggle_pause()
-            elif options.nodbsize and key == keys.REFRESH_DB_SIZE:
+            elif not options.dbsize and key == keys.REFRESH_DB_SIZE:
                 skip_db_size = False
             elif key is not None:
                 if keys.is_process_next(key):
@@ -184,15 +184,15 @@ def main(
 
             else:
                 if not ui.in_pause and not ui.interactive():
-                    if options.nodbsize and not skip_db_size:
+                    if not options.dbsize and not skip_db_size:
                         skip_db_size = True
 
                     server_information = data.pg_get_server_information(
                         prev_server_info=server_information,
                         using_rds=options.rds,
                         skip_db_size=skip_db_size,
-                        skip_tempfile=options.notempfiles,
-                        skip_walreceiver=options.nowalreceiver,
+                        skip_tempfile=not options.tempfiles,
+                        skip_walreceiver=not options.walreceiver,
                     )
                     memory, swap, load = activities.mem_swap_load()
                     system_info = types.SystemInfo.default(
diff --git a/tests/test_cli.txt b/tests/test_cli.txt
@@ -81,4 +81,4 @@ libpq 9.2).
 
 >>> ns = parser.parse_args(["--no-db-size", "-w", "-p", "5433", "--no-pid", "--no-app-name"])
 >>> vars(ns)
-{'profile': None, 'blocksize': 4096, 'rds': False, 'output': None, 'nodbsize': True, 'notempfiles': False, 'nowalreceiver': False, 'wrap_query': True, 'durationmode': '1', 'minduration': 0, 'filters': [], 'debug_file': None, 'help': False, 'connection_string': '', 'host': None, 'port': '5433', 'username': None, 'dbname': None, 'nopid': True, 'nodb': False, 'nouser': False, 'noclient': False, 'nocpu': False, 'nomem': False, 'noread': False, 'nowrite': False, 'notime': False, 'nowait': False, 'noappname': True, 'header_show_instance': None, 'header_show_system': None, 'header_show_workers': None, 'hide_queries_in_logs': False, 'refresh': 2}
+{'profile': None, 'blocksize': 4096, 'rds': False, 'output': None, 'dbsize': False, 'tempfiles': True, 'walreceiver': True, 'wrap_query': True, 'durationmode': '1', 'minduration': 0, 'filters': [], 'debug_file': None, 'help': False, 'connection_string': '', 'host': None, 'port': '5433', 'username': None, 'dbname': None, 'pid': False, 'database': True, 'user': True, 'client': True, 'cpu': True, 'mem': True, 'read': True, 'write': True, 'time': True, 'wait': True, 'appname': False, 'header_show_instance': None, 'header_show_system': None, 'header_show_workers': None, 'hide_queries_in_logs': False, 'refresh': 2}
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -19,17 +19,17 @@ def test_flag():
 
 def test_flag_load():
     options = {
-        "noappname": False,
-        "noclient": False,
-        "nocpu": False,
-        "nodb": False,
-        "nomem": False,
-        "nopid": False,
-        "noread": False,
-        "notime": False,
-        "nouser": False,
-        "nowait": False,
-        "nowrite": False,
+        "appname": True,
+        "client": True,
+        "cpu": True,
+        "database": True,
+        "mem": True,
+        "pid": True,
+        "read": True,
+        "time": True,
+        "user": True,
+        "wait": True,
+        "write": True,
     }
     flag = Flag.load(None, is_local=True, **options)
     assert (
@@ -67,9 +67,9 @@ def test_flag_load():
         | Flag.APPNAME
         | Flag.DATABASE
     )
-    options["nodb"] = True
-    options["notime"] = True
-    options["nopid"] = True
+    options["database"] = False
+    options["time"] = False
+    options["pid"] = False
     cfg = Configuration(
         name="test",
         values=dict(database=UISection(hidden=False), relation=UISection(hidden=True)),
diff --git a/tests/test_ui.txt b/tests/test_ui.txt
@@ -26,21 +26,21 @@ Default CLI options, passed to ui.main():
 ...     "refresh": 2,
 ...     "minduration": 0,
 ...     "filters": [],
-...     "noappname": True,
-...     "noclient": False,
-...     "nocolor": False,
-...     "nocpu": True,
-...     "nodb": False,
-...     "nodbsize": False,
-...     "notempfiles": False,
-...     "nowalreceiver": False,
-...     "nomem": True,
-...     "nopid": False,
-...     "noread": True,
-...     "notime": True,
-...     "nouser": False,
-...     "nowait": True,
-...     "nowrite": True,
+...     "appname": False,
+...     "client": True,
+...     "color": True,
+...     "cpu": False,
+...     "database": True,
+...     "dbsize": True,
+...     "tempfiles": True,
+...     "walreceiver": True,
+...     "mem": False,
+...     "pid": True,
+...     "read": False,
+...     "time": False,
+...     "user": True,
+...     "wait": False,
+...     "write": False,
 ...     "output": None,
 ...     "port": f"{postgres.info.port}",
 ...     "rds": False,
@@ -431,7 +431,7 @@ PID    DATABASE              USER       CLIENT             state Query
 <BLANKLINE>
 ------------------------------------------------------------------------------------------- sending key 'q' --------------------------------------------------------------------------------------------
 
->>> defaults["nopid"] = True
+>>> defaults["pid"] = False
 >>> defaults["header_show_instance"] = False
 >>> defaults["header_show_workers"] = False
 >>> defaults["header_show_system"] = False
@@ -823,10 +823,10 @@ Use another set of options:
 ...         defaults,
 ...         durationmode="2",
 ...         wrap_query=True,
-...         noclient=True,
-...         nodbsize=True,
-...         notempfiles=False,
-...         nowalreceiver=False,
+...         client=False,
+...         dbsize=False,
+...         tempfiles=True,
+...         walreceiver=True,
 ...         output=str(output),
 ...     ),
 ... )
@@ -940,9 +940,9 @@ Interactive mode:
 ...         defaults,
 ...         durationmode="1",
 ...         wrap_query=True,
-...         nopid=False,
-...         noclient=True,
-...         nouser=True,
+...         pid=True,
+...         client=False,
+...         user=False,
 ...         output=None,
 ...     ),
 ... )
