feat: begin adding settings menu #647

python357-1 · 2024-12-15T23:53:10Z

This PR addresses the "Settings Menu" item in the tagstudio roadmap

python357-1 · 2025-01-11T06:54:41Z

tagstudio/src/qt/translations.py

@@ -30,7 +30,7 @@ def value(self) -> str:

    @value.setter
    def value(self, value: str):
-        if self.__value != value:
+        if self.__value != value and value is not None:


@Computerdores I'm sure this is not at all an ideal solution to this problem, but it seems to fix the problem I was having with changing the language while TagStudio is running and having any non-translated elements have no text. If you have a better way of fixing this, I'm all ears

Yeah you are right it is definitely not ideal as it would introduce another bug. I have an idea for a proper fix - I'll give this PR a review later and include the fix there.

python357-1 · 2025-01-11T06:57:53Z

@CyanVoxel After a very quick 4 weeks of development, the extremely basic settings menu is complete and ready for review

(sarcasm) but seriously it is ready for review

Computerdores

Issues I noticed while testing:

The view menu is empty so clicking it doesn't do anything (should probably be removed)
Changing the "Show filenames" option has no effect without restarting TS
~~Changing the language has no effect (at least changing to es-MX doesn't)~~
~~The Dark mode setting doesn't work for me (Windows 10, Windows is set to dark mode, TS is set to light mode; TS shows in dark mode anyways)~~

tagstudio/src/qt/translations.py

Computerdores · 2025-01-11T12:23:23Z

tagstudio/src/core/settings/tssettings.py

+        if isinstance(path, str):
+            path_value = Path(path)
+
+        if path is None:
+            path_value = Path(self.filename)


The second if can be an else instead - it would be more intuitive that way

Suggested change

if isinstance(path, str):

path_value = Path(path)

if path is None:

path_value = Path(self.filename)

if isinstance(path, str):

path_value = Path(path)

else:

path_value = Path(self.filename)

Computerdores · 2025-01-11T12:25:24Z

tagstudio/src/core/settings/libsettings.py

+    # Cant think of any library-specific properties lol
+    test_prop: bool = False


There is the list of excluded file extensions that could be here, no?

Computerdores · 2025-01-11T12:42:45Z

tagstudio/src/core/tscacheddata.py

+            if not Path(path).exists():
+                logger.info("Cache file does not exist - creating", path=path)
+                open(path, "w").close()


This will fail if the parent directory doesn't exist. I would suggest taking the logic from the if path is None: case and using it for both cases with the path being chosen based on whether path is None

Computerdores · 2025-01-11T13:08:15Z

tagstudio/src/qt/ts_qt.py

+        settings_menu_action = QAction("&Settings", menu_bar)
+        settings_menu_action.triggered.connect(lambda: self.open_settings_menu())
+        edit_menu.addAction(settings_menu_action)
+
+        edit_menu.addSeparator()
+


I would put this under "File" instead of "Edit" since all the other options under "Edit" are about editing the currently open library, while all the options in the Settings menu are unrelated to the library.

Computerdores · 2025-01-11T13:27:10Z

tagstudio/src/qt/widgets/preview_panel.py

+    # def fill_libs_widget(self, layout: QVBoxLayout):
+    #    settings = self.driver.settings
+    #    settings.beginGroup(SettingItems.LIBS_LIST)
+    #    lib_items: dict[str, tuple[str, str]] = {}
+    #    for item_tstamp in settings.allKeys():
+    #        val = str(settings.value(item_tstamp, type=str))
+    #        cut_val = val
+    #        if len(val) > 45:
+    #            cut_val = f"{val[0:10]} ... {val[-10:]}"
+    #        lib_items[item_tstamp] = (val, cut_val)
+
+    #    settings.endGroup()
+
+    #    new_keys = set(lib_items.keys())
+    #    if new_keys == self.render_libs:
+    #        # no need to re-render
+    #        return
+
+    #    # sort lib_items by the key
+    #    libs_sorted = sorted(lib_items.items(), key=lambda item: item[0], reverse=True)
+
+    #    self.render_libs = new_keys
+    #    self._fill_libs_widget(libs_sorted, layout)
+


this can be removed, no?

Computerdores · 2025-01-11T13:30:38Z

tagstudio/tests/fixtures/search_library/.TagStudio/ts_library.sqlite

What did you change about this file?

python357-1 · 2025-01-11T17:49:31Z

just to be sure - are you on the most recent commit? es-MX should not be an option in the language list, and dark mode is not in the settings anymore

Computerdores · 2025-01-11T18:06:36Z

just to be sure - are you on the most recent commit? es-MX should not be an option in the language list, and dark mode is not in the settings anymore

god dammit - let me fetch real quick

Computerdores · 2025-01-11T18:12:15Z

I updated my comment - the dark mode and language thing were due to me forgetting to fetch, but the other two are still applicable

python357-1 · 2025-01-13T07:05:50Z

everything should be addressed

Computerdores

Most stuff indeed seems to be fixed.

Notes:

search_library is still reported by GitHub to have been changed
You have added some strings in this PR that aren't using the Translation system
(There is still the commented out stuff in preview_panel, but that is more of a nit anyways)

tagstudio/src/core/settings/tssettings.py

VasigaranAndAngel

in my review, i've included fixes for some type issues reported by mypy/pyright, along with a few doubts i have and some minor nitpicks and suggestions.

There are three classes inherited from BaseModel (TSSettings, LibSettings, TSCachedData) that have almost the same methods. what if it's modularized?
Can you add docstrings for methods

VasigaranAndAngel · 2025-01-13T03:14:20Z

tagstudio/src/core/settings/tssettings.py

+                if len(filecontents.strip()) != 0:
+                    settings_data = toml.loads(filecontents.decode("utf-8"))
+
+        settings_data["filename"] = str(path)


what is the purpose of saving the file path of it's own?

tagstudio/src/core/tscacheddata.py

requirements.txt

tagstudio/src/core/settings/libsettings.py

tagstudio/src/core/library/alchemy/library.py

tagstudio/src/qt/ts_qt.py

CyanVoxel added Type: UI/UX User interface and/or user experience Priority: High An important issue requiring attention labels Dec 16, 2024

CyanVoxel added this to the Alpha v9.5 (Post-SQL) milestone Dec 16, 2024

This comment was marked as outdated.

Sign in to view

python357-1 force-pushed the add_settings_menu branch from dcf34fd to 037ebde Compare December 24, 2024 02:19

python357-1 force-pushed the add_settings_menu branch from 18989ff to e3d90a7 Compare January 1, 2025 00:56

python357-1 commented Jan 11, 2025

View reviewed changes

python357-1 marked this pull request as ready for review January 11, 2025 06:54

python357-1 added the Status: Review Needed A review of this is needed label Jan 11, 2025

Computerdores suggested changes Jan 11, 2025

View reviewed changes

feat: add settings menu

854c805

Computerdores reviewed Jan 13, 2025

View reviewed changes

tagstudio/src/core/settings/tssettings.py Outdated Show resolved Hide resolved

VasigaranAndAngel suggested changes Jan 13, 2025

View reviewed changes

fix: merge conflicts and review suggestions

b74e872

python357-1 force-pushed the add_settings_menu branch from 3fae2e6 to b74e872 Compare January 14, 2025 05:58

fix: review suggestion

70a6c15

This was referenced Jan 15, 2025

feat(ui): add configurable splash screens #703

Open

feat(ui): add thumbnail caching #694

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: begin adding settings menu #647

feat: begin adding settings menu #647

python357-1 commented Dec 15, 2024

This comment was marked as outdated.

python357-1 Jan 11, 2025

Computerdores Jan 11, 2025

python357-1 commented Jan 11, 2025

Computerdores left a comment •

edited

Loading

Computerdores Jan 11, 2025

Computerdores Jan 11, 2025

Computerdores Jan 11, 2025

Computerdores Jan 11, 2025

Computerdores Jan 11, 2025

Computerdores Jan 11, 2025

python357-1 commented Jan 11, 2025

Computerdores commented Jan 11, 2025 •

edited

Loading

Computerdores commented Jan 11, 2025

python357-1 commented Jan 13, 2025

Computerdores left a comment

VasigaranAndAngel left a comment

VasigaranAndAngel Jan 13, 2025

		# Cant think of any library-specific properties lol
		test_prop: bool = False

feat: begin adding settings menu #647

Are you sure you want to change the base?

feat: begin adding settings menu #647

Conversation

python357-1 commented Dec 15, 2024

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

python357-1 commented Jan 11, 2025

Computerdores left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

python357-1 commented Jan 11, 2025

Computerdores commented Jan 11, 2025 • edited Loading

Computerdores commented Jan 11, 2025

python357-1 commented Jan 13, 2025

Computerdores left a comment

Choose a reason for hiding this comment

VasigaranAndAngel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Computerdores left a comment •

edited

Loading

Computerdores commented Jan 11, 2025 •

edited

Loading