From 087f1aa937668c6eef163fccbdfc0088f5b98f0c Mon Sep 17 00:00:00 2001
From: Shawn Brown <shawnbrown@users.noreply.github.com>
Date: Sat, 11 Sep 2021 13:10:41 -0400
Subject: [PATCH] Update `rcfile` and `banner` normalization.

Also remove tests for bogus assignment and add type hints instead.

This change moves the normalization from the property setters into
the getters. Doing this makes sure we are checking the data as it
comes in to the application rather than when it leaves the app.

This also means that we are trusting data generated from within
the app. And the type hints have been added to help check that
the implementation is correct.
---
 envlauncher/app.py | 21 +++++++++------------
 tests/test_app.py  | 12 ++++--------
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/envlauncher/app.py b/envlauncher/app.py
index 38218f1..2c29eae 100644
--- a/envlauncher/app.py
+++ b/envlauncher/app.py
@@ -155,8 +155,6 @@ def __init__(self, file_or_path):
         self._parser.read_string(string)
 
         self._terminal_emulator_choices = get_terminal_emulators()
-        self._rcfile = self._parser.get('X-EnvLauncher Options', 'Rcfile', fallback='')
-        self._banner = self._parser.get('X-EnvLauncher Options', 'Banner', fallback='color')
         self._venv_number = itertools.count(1)
         self._app_data_subdir = 'envlauncher'
 
@@ -209,13 +207,11 @@ def rcfile(self) -> str:
         """An "rc" file to execute after activating the environment
         (e.g., ~/.bashrc).
         """
-        return self._rcfile
+        return self._parser.get('X-EnvLauncher Options', 'Rcfile', fallback='')
 
     @rcfile.setter
-    def rcfile(self, value):
-        if not isinstance(value, str):
-            value = ''
-        self._rcfile = value
+    def rcfile(self, value: str):
+        self._parser['X-EnvLauncher Options']['Rcfile'] = value
 
     @property
     def terminal_emulator_choices(self) -> List[str]:
@@ -239,13 +235,14 @@ def terminal_emulator(self, value):
     @property
     def banner(self) -> str:
         """Python logo banner option."""
-        return self._banner
+        value = self._parser.get('X-EnvLauncher Options', 'Banner', fallback='')
+        if value not in {'color', 'plain', 'none'}:
+            return 'color'
+        return value
 
     @banner.setter
-    def banner(self, value):
-        if value not in {'color', 'plain', 'none'}:
-            value = 'color'
-        self._banner = value
+    def banner(self, value: str):
+        self._parser['X-EnvLauncher Options']['Banner'] = value
 
     @property
     def banner_resource(self) -> Optional[Tuple[str, str]]:
diff --git a/tests/test_app.py b/tests/test_app.py
index b8bcfb7..ef34b2e 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -259,10 +259,6 @@ def test_set_value(self):
         self.settings.rcfile = '.venvrc'
         self.assertEqual(self.settings.rcfile, '.venvrc')
 
-    def test_invalid_value(self):
-        self.settings.rcfile = 1234  # <- Bogus value.
-        self.assertEqual(self.settings.rcfile, '')  # <- Empty string.
-
 
 class TestSettingsTerminalEmulator(unittest.TestCase):
     def setUp(self):
@@ -309,6 +305,10 @@ def setUp(self):
     def test_read(self):
         self.assertEqual(self.settings.banner, 'color')
 
+    def test_read_bad_value(self):
+        self.settings.banner = 'some bad value'
+        self.assertEqual(self.settings.banner, 'color')
+
     def test_set_value(self):
         self.settings.banner = 'plain'
         self.assertEqual(self.settings.banner, 'plain')
@@ -316,10 +316,6 @@ def test_set_value(self):
         self.settings.banner = 'none'
         self.assertEqual(self.settings.banner, 'none')
 
-    def test_invalid_value(self):
-        self.settings.banner = 1234  # <- Bogus value.
-        self.assertEqual(self.settings.banner, 'color')  # <- Defaults to color.
-
     def test_banner_resource(self):
         self.assertEqual(self.settings.banner_resource,
                          ('envlauncher', 'banner-color.ascii'))