files-community / Files

Building the best file manager for Windows
https://files.community
MIT License
33.3k stars 2.13k forks source link

Code Quality: Use seperate json file for tabs list #14875

Open yaira2 opened 5 months ago

yaira2 commented 5 months ago

Description

We currently update the settings file whenever there is a change to the tab list. We do this to preserve the list of tabs in the event of an improper shutdown. At the same time, this requires serializing the list of settings into json when all we need is to save the list of tabs.

Concerned code

AppLifecycleHelper.SaveSessionTabs();

Gains

Requirements

Comments

No response

d2dyno1 commented 5 months ago

The only challenging bit of this issue is migrating existing tabs to the new format

yaira2 commented 5 months ago

The only challenging bit of this issue is migrating existing tabs to the new format

How about checking if there is an existing list and copying them to the new format, once the existing list is empty we can ignore the check moving forward.

saikotek commented 5 months ago

Hello, I think I could do it.

From what I can see you share singleton instance of UserSettingsService across all classes of settings by passing ISettingsSharingContext to them. This means that property userSettingsService.GeneralSettingsService.LastSessionTabList uses the same UserSettingsService singleton which saves data through CachingJsonSettingsDatabase. I could register a new service just like FileTagsSettingsService to save to its own file, right?

If my understanding is correct then I can attempt to do it :)

yaira2 commented 5 months ago

@d2dyno1 is that right?

0x5bfa commented 2 months ago

Yes, correct.

0x5bfa commented 1 month ago

In order to avoid using left and right wording, we might as well use a list instead.

[
  {
    "ShellPanes":
    [
      "C:\\Users",
      "C:\\Windows",
    ]
    "ShellPaneArrangement": 0
  },
  {
    "ShellPanes":
    [
      "C:\\Windows\\System32",
      "C:\\Program Files (x86)",
    ]
    "ShellPaneArrangement": 1
  },
  {
    "ShellPanes":
    [
      "C:\\Users\\0x5BFA\\Desktop",
      "C:\\Users\\0x5BFA\\AppData\\Local",
    ]
    "ShellPaneArrangement": 0
  }
]
yaira2 commented 1 month ago

Something like that 👍