Code refactor (#689)

* Codebase Refactor I scanned the entire codebase for potential issues regarding performance and readability and fixed as many issues as possible. This should make the codebase "modern" and more cleaner to read/review through. These fixes include: - Better named expressions added - f-strings added rather than interpolated variable names better conditional branching added (which is a good practice and follows standards laid out by the Python community) - code made more readable following "pythonic" code style - improved formatting based on PEP8 guidelines - added list comprehensions wherever needed replacing large, complex-looking code blocks - `TODO.md` formatted based on Markdown file guidelines. --------- Co-authored-by: HighnessAtharva <HighnessAtharva@gmail.com> Co-authored-by: Jendrik Seipp <jendrikseipp@gmail.com>
jendrikseipp · May 11, 2023 · accaa0e · accaa0e
1 parent a120b96
commit accaa0e
Show file tree

Hide file tree

Showing 25 changed files with 152 additions and 223 deletions.
diff --git a/.gitignore b/.gitignore
@@ -7,6 +7,7 @@ po/*.mo
 .pc/
 .pybuild/
 .tox/
+rednotebook.egg-info
 debian/rednotebook/
 debian/debhelper-build-stamp
 debian/files

diff --git a/TODO.md b/TODO.md
@@ -4,14 +4,13 @@ If you have any suggestions or comments, feel free to contact me on the
 mailing list or directly per mail. Before starting to work on a feature,
 please check back with me briefly about its status.
 
-
 ## Important features
 
 - [ ] Update GTK stack for Windows: use MinGW and Python >= 3.6.
 - [ ] Use separate file for storing CSS to allow users to override styles more easily.
 - [ ] Make default CSS prettier (see private email exchange).
 - [ ] Allow searching for days that contain **multiple** words or tags.
-- [ ] Check that non-ASCII image filenames work (https://bugs.launchpad.net/bugs/1739701).
+- [ ] Check that non-ASCII image filenames work (<https://bugs.launchpad.net/bugs/1739701>).
 - [ ] Search and replace (useful for renaming tags and other names).
       Show "replace" text field after search text has been entered.
 - [ ] Add simple way to show all entries: allow searching for whitespace (i.e., don't strip whitespace from search string).
@@ -28,7 +27,6 @@ please check back with me briefly about its status.
   - [ ] Check that images work on Windows in LaTeX exports.
   - [ ] Fix drag-and-drop on Windows (confirmed that it doesn't work in version 2.6.1). Might be fixed by now.
 
-
 ### Remove right-side tags panel (disabled by default)
 
 - [ ] When searching for a hashtag (see #498):
@@ -39,7 +37,6 @@ please check back with me briefly about its status.
 - [ ] Transform existing right-side tags foo:bar to "#foo bar" when loading a journal.
 - [ ] Remove code for right-side tags panel.
 
-
 ## Optional features
 
 - [ ] Auto-completion for hashtags.
@@ -59,14 +56,12 @@ please check back with me briefly about its status.
   - only show the menu if there's a selection: WebView.has_selection() seems to be broken
 - [ ] Translate templates.
 
-
 ## Implementation changes
 
-- [ ] Enable faulthandler module (https://docs.python.org/3/library/faulthandler.html, added in Python 3.3).
+- [ ] Enable faulthandler module (<https://docs.python.org/3/library/faulthandler.html>, added in Python 3.3).
 - [ ] Don't store regexes in Cloud class.
 - [ ] Clouds: don't store link_dict, but use names directly in HTML in Cloud class.
 
-
 ## Deferred features
 
 After deciding whether to switch to Markdown or not:
@@ -81,7 +76,6 @@ After deciding whether to switch to Markdown or not:
 - [X] Numbered lists
 - [X] Add quotes by indenting them with a tab
 
-
 ## Unwanted features
 
 - verbatim / raw ( """/"" - supported, undocumented) (too confusing / poorly behaving)

diff --git a/dev/generate-pot.sh b/dev/generate-pot.sh
@@ -40,7 +40,7 @@ xgettext    --output=rednotebook.pot \
             --files-from=sourcefiles.txt \
             tmp/main_window.glade.h
 
-for file in `ls *.po`; do
+for file in $(ls *.po); do
     msgmerge --previous --update ${file} rednotebook.pot
 done
 

diff --git a/rednotebook/configuration.py b/rednotebook/configuration.py
@@ -23,9 +23,7 @@
 
 
 def delete_comment(line):
-    if line.startswith("#"):
-        return ""
-    return line
+    return "" if line.startswith("#") else line
 
 
 class Config(dict):
@@ -139,17 +137,17 @@ def write_list(self, key, list):
         self[key] = ", ".join(list)
 
     def changed(self):
-        return not (self == self.old_config)
+        return self != self.old_config
 
     def save_to_disk(self):
         if not self.changed():
             return
 
-        lines = []
-        for key, value in sorted(self.items()):
-            if key not in self.suppressed_keys:
-                lines.append(f"{key}={value}")
-
+        lines = [
+            f"{key}={value}"
+            for key, value in sorted(self.items())
+            if key not in self.suppressed_keys
+        ]
         try:
             filesystem.make_directory(os.path.dirname(self.filename))
             filesystem.write_file(self.filename, "\n".join(lines))
@@ -158,5 +156,5 @@ def save_to_disk(self):
                 "Configuration could not be saved. Please check " "your permissions"
             )
         else:
-            logging.info("Configuration has been saved to %s" % self.filename)
+            logging.info(f"Configuration has been saved to {self.filename}")
             self.save_state()
diff --git a/rednotebook/data.py b/rednotebook/data.py
@@ -78,7 +78,7 @@ def get_text_with_dots(text, start, end, found_text=None):
     res = res.replace("\n", " ")
     if found_text:
         # Make the searched_text bold
-        res = res.replace(found_text, "STARTBOLD%sENDBOLD" % found_text)
+        res = res.replace(found_text, f"STARTBOLD{found_text}ENDBOLD")
 
     return res
 
@@ -165,7 +165,7 @@ def get_words(self, with_special_chars=False):
             for category, content in self.get_category_content_pairs().items()
         )
 
-        all_text = self.text + " " + categories_text
+        all_text = f"{self.text} {categories_text}"
         words = all_text.split()
 
         if with_special_chars:
@@ -204,8 +204,7 @@ def search(self, text, tags):
             # Date contains searched text.
             results.append(get_text_with_dots(self.text, 0, TEXT_RESULT_LENGTH))
         else:
-            text_result = self.search_in_text(text)
-            if text_result:
+            if text_result := self.search_in_text(text):
                 results.append(text_result)
             results.extend(self.search_in_categories(text))
         return str(self), results
@@ -218,10 +217,9 @@ def search_in_text(self, search_text):
             return None
 
         found_text = self.text[occurrence : occurrence + len(search_text)]
-        result_text = get_text_with_dots(
+        return get_text_with_dots(
             self.text, occurrence, occurrence + len(search_text), found_text
         )
-        return result_text
 
     def search_in_categories(self, text):
         results = []
@@ -261,8 +259,9 @@ def get_day(self, day_number):
 
     def __str__(self):
         lines = [f"Month {self.year_number} {self.month_number}"]
-        for day_number, day in self.days.items():
-            lines.append(f"{day_number}: {day.text}")
+        lines.extend(
+            f"{day_number}: {day.text}" for day_number, day in self.days.items()
+        )
         return "\n".join(lines)
 
     @property

diff --git a/rednotebook/gui/browser.py b/rednotebook/gui/browser.py
@@ -41,10 +41,7 @@ def __init__(self):
             self.show_all()
 
         def set_font_size(self, size):
-            if size <= 0:
-                zoom = 1.0
-            else:
-                zoom = size / 10.0
+            zoom = 1.0 if size <= 0 else size / 10.0
             # It seems webkit shows text a little bit bigger.
             zoom *= 0.90
             self.set_zoom_level(zoom)

diff --git a/rednotebook/gui/categories.py b/rednotebook/gui/categories.py
@@ -99,7 +99,7 @@ def add_category(self, category):
         self.categories.sort(key=utils.sort_asc)
 
     def node_on_top_level(self, iter):
-        if not type(iter) == Gtk.TreeIter:
+        if type(iter) != Gtk.TreeIter:
             # iter is a path -> convert to iter
             iter = self.tree_store.get_iter(iter)
         assert self.tree_store.iter_is_valid(iter)
@@ -167,32 +167,26 @@ def set_day_content(self, day):
         sorted_keys = sorted(day.content.keys(), key=lambda x: x.lower())
 
         for key in sorted_keys:
-            value = day.content[key]
-            if not key == "text":
+            if key != "text":
+                value = day.content[key]
                 self.add_element(None, {key: value})
         self.tree_view.expand_all()
 
     def get_day_content(self):
-        if self.empty():
-            return {}
-
-        content = self._get_element_content(None)
-
-        return content
+        return {} if self.empty() else self._get_element_content(None)
 
     def _get_element_content(self, element):
         model = self.tree_store
         if self.tree_store.iter_n_children(element) == 0:
             return None
-        else:
-            content = {}
+        content = {}
 
-            for i in range(model.iter_n_children(element)):
-                child = model.iter_nth_child(element, i)
-                txt2tags_markup = self.get_iter_value(child)
-                content[txt2tags_markup] = self._get_element_content(child)
+        for i in range(model.iter_n_children(element)):
+            child = model.iter_nth_child(element, i)
+            txt2tags_markup = self.get_iter_value(child)
+            content[txt2tags_markup] = self._get_element_content(child)
 
-            return content
+        return content
 
     def empty(self, category_iter=None):
         """
@@ -217,9 +211,7 @@ def get_iter_value(self, iter):
         self.tvcolumn.clear_attributes(self.cell)
         self.tvcolumn.add_attribute(self.cell, "markup", 0)
 
-        # We want to have txt2tags markup and not pango markup
-        text = convert_from_pango(pango_markup)
-        return text
+        return convert_from_pango(pango_markup)
 
     def set_iter_value(self, iter, txt2tags_markup):
         pango_markup = convert_to_pango(txt2tags_markup)
@@ -233,7 +225,7 @@ def _get_category_iter(self, category_name):
                 return current_category_iter
 
         # If the category was not found, return None
-        logging.debug('Category not found: "%s"' % category_name)
+        logging.debug(f'Category not found: "{category_name}"')
         return None
 
     def add_entry(self, category, entry, undoing=False):
@@ -277,8 +269,7 @@ def delete_node(self, iter, undoing=False):
         self.main_window.cloud.update()
 
     def delete_selected_node(self):
-        selected_iter = self.get_selected_node()
-        if selected_iter:
+        if selected_iter := self.get_selected_node():
             self.delete_node(selected_iter)
             return
 
@@ -361,9 +352,7 @@ def _get_context_menu(self):
         # Add a UI description
         uimanager.add_ui_from_string(context_menu_xml)
 
-        # Create a Menu
-        menu = uimanager.get_widget("/ContextMenu")
-        return menu
+        return uimanager.get_widget("/ContextMenu")
 
     def _on_change_entry_clicked(self, action):
         iter = self.get_selected_node()

diff --git a/rednotebook/gui/clouds.py b/rednotebook/gui/clouds.py
@@ -53,9 +53,9 @@
 
 def get_regex(word):
     try:
-        return re.compile(word + "$", re.I)
+        return re.compile(f"{word}$", re.I)
     except Exception:
-        logging.warning('"%s" is not a valid regular expression' % word)
+        logging.warning(f'"{word}" is not a valid regular expression')
         return re.compile("^$")
 
 
@@ -104,7 +104,7 @@ def get_categories_counter(self):
         counter = defaultdict(int)
         for day in self.journal.days:
             for cat in day.categories:
-                counter["#%s" % data.escape_tag(cat)] += 1
+                counter[f"#{data.escape_tag(cat)}"] += 1
         return counter
 
     def _update(self):
@@ -138,20 +138,16 @@ def _get_cloud_body(self, cloud_words):
 
         min_font_size = 10
         max_font_size = 40
-
         font_delta = max_font_size - min_font_size
-
         html_elements = []
-
         for word, count in cloud_words:
             font_factor = (count - min_count) / delta_count
             font_size = int(min_font_size + font_factor * font_delta)
 
             # Add some whitespace to separate words
             html_elements.append(
-                '<a href="/#search-%s">'
-                '<span style="font-size:%spx">%s</span></a>&#160;'
-                % (self.link_index, font_size, word)
+                f'<a href="/#search-{self.link_index}"><span '
+                f'style="font-size:{font_size}px">{word}</span></a>&#160;'
             )
             self.link_index += 1
         return "\n".join(html_elements)
@@ -233,7 +229,7 @@ def on_decide_policy(self, webview, decision, decision_type):
 
             search_text = self._get_search_text(uri)
             if search_text is not None:
-                logging.info('Clicked cloud URI "%s"' % uri)
+                logging.info(f'Clicked cloud URI "{uri}"')
                 self.journal.save_old_day()
                 self.journal.frame.search_box.set_active_text(search_text)
                 self.journal.frame.search_box.search(search_text)

diff --git a/rednotebook/gui/customwidgets.py b/rednotebook/gui/customwidgets.py
@@ -163,7 +163,7 @@ def show_message(self, title, msg, msg_type):
         if not title:
             title = msg
             msg = ""
-        self.title_label.set_markup("<b>%s</b>" % title)
+        self.title_label.set_markup(f"<b>{title}</b>")
         self.msg_label.set_markup(msg)
         self.set_message_type(msg_type)
         self.image.set_from_icon_name(
@@ -256,9 +256,7 @@ def prepare(self, porter):
         self.path_type = porter.PATHTYPE.upper()
         path = porter.DEFAULTPATH
         extension = porter.EXTENSION
-        helptext = porter.PATHTEXT
-
-        if helptext:
+        if helptext := porter.PATHTEXT:
             self.set_header(helptext)
 
         if self.path_type == "DIR":
@@ -268,12 +266,12 @@ def prepare(self, porter):
         elif self.path_type == "NEWFILE":
             self.chooser.set_action(Gtk.FileChooserAction.SAVE)
         else:
-            logging.error('Wrong path_type "%s"' % self.path_type)
+            logging.error(f'Wrong path_type "{self.path_type}"')
 
         if self.path_type in ["FILE", "NEWFILE"] and extension:
             filter = Gtk.FileFilter()
             filter.set_name(extension)
-            filter.add_pattern("*." + extension)
+            filter.add_pattern(f"*.{extension}")
             self.chooser.add_filter(filter)
 
         if self.last_path and os.path.exists(self.last_path):
@@ -285,7 +283,7 @@ def prepare(self, porter):
             dirname, basename = os.path.split(path)
             filename, _ = os.path.splitext(basename)
             self.chooser.set_current_folder(dirname)
-            self.chooser.set_current_name(filename + "." + extension)
+            self.chooser.set_current_name(f"{filename}.{extension}")
 
     def get_selected_path(self):
         self.last_path = self.chooser.get_filename()
@@ -333,7 +331,7 @@ class TemplateBar(Gtk.HBox):
     def __init__(self):
         GObject.GObject.__init__(self)
         self.set_spacing(2)
-        label = Gtk.Label(label="<b>%s</b>:" % _("Template"))
+        label = Gtk.Label(label=f'<b>{_("Template")}</b>:')
         label.set_use_markup(True)
         self.pack_start(label, False, False, 0)
         self.save_insert_button = Gtk.Button.new_with_label(_("Save and insert"))

diff --git a/rednotebook/gui/editor.py b/rednotebook/gui/editor.py
@@ -63,8 +63,8 @@ def __init__(self, day_text_view):
 
         self.font = Pango.FontDescription(DEFAULT_FONT)
         self.default_size = self.font.get_size() / Pango.SCALE
-        logging.debug("Default font: %s" % self.font.to_string())
-        logging.debug("Default size: %s" % self.default_size)
+        logging.debug(f"Default font: {self.font.to_string()}")
+        logging.debug(f"Default size: {self.default_size}")
 
     def replace_buffer(self, buffer):
         self.day_text_view.set_buffer(buffer)
@@ -163,8 +163,7 @@ def scroll_to_text(self, text):
             return  # Stop after the first match
 
     def get_selected_text(self):
-        bounds = self.day_text_buffer.get_selection_bounds()
-        if bounds:
+        if bounds := self.day_text_buffer.get_selection_bounds():
             return self.get_text(*bounds)
         else:
             return ""