KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
391 stars 33 forks source link

Add support for project-level and part-level settings #1503

Closed franknoirot closed 6 months ago

franknoirot commented 7 months ago

Currently we have only one block of settings that are shared between all projects and files. As @JBEmbedded pointed out, this can lead to confusing behavior if a user thinks they are setting something like the Base Units for their project to Centimeters, then come to find that their units have changed in another project they meant to keep in Inches.

### Tasks
- [x] Create an architecture plan, looking at VS Code and Obsidian's config systems as inspiration
- [ ] Create a way to define user-level settings, project-level settings, and part-level settings
- [ ] Make it possible to read and write these settings from files (#1361 extended)
franknoirot commented 7 months ago

Relevant research items:

  1. https://code.visualstudio.com/docs/getstarted/settings
  2. https://help.obsidian.md/Files+and+folders/How+Obsidian+stores+data#Vault+settings
franknoirot commented 7 months ago

More relevant research: a substantial number of obsidian power users hate that it defaults to per-vault settings, and that it lacks a way to set global defaults for new vaults (our equivalent is projects).

franknoirot commented 7 months ago

I've made a Figma mockup flow here. Here's the most important screen from it:

Step 4

Requirements

franknoirot commented 7 months ago

Starting implementation of this now, here's my plan.

An example

Here is an example settings object (the context of settingsMachine) after reading in the default, user, project, and part-level settings files:

const settings = {
  units: {
    length: {
      default: "millimeters",
      user: "inches",
      part: "millimeters",
    },
    angle: {
      default: "degree",
      project: "radians",
    },
  },
  editor: {
    mouseControls: {
      default: "Zoo",
    },
  },
}

In the example above, the user has the following settings files on their desktop app. Note that these will likely be .json files, but I'm showing them as JS settings constants for comparability to the object above:

in $APP_DATA/settings.json:

const settings = {
  units: {
    length: "inches",
  },
}

in <some-zoo-project-dir>/settings.json:

const settings = {
  units: {
    angle: "radians",
  },
}

and in <some-zoo-project-dir>/<part-name>.settings.json:

const settings = {
  units: {
    length: "millimeters",
  },
}

So the settings are applied as follows:

  1. No overrides are saved for mouseControls, so we use the default "Zoo" value
  2. A project-level override is provided for units.angle, so we use "radians" instead of the default "degrees"
  3. Both a user- and part-level override are provided for units.length, so we use the part-level override that takes precedence.
franknoirot commented 7 months ago

If we're going to write to and read from a settings file within the current project's directory and using the current part's name, we have to find a way to resolve the fact that currently the settingsMachine doesn't know about the existence of the fileMachine or indeed the currently-open project or part at all.

I think that we need to do the work of #1248 and then we can implement an actor system to avoid this "which machine is truly global" and let them coordinate across each other responsibly.

franknoirot commented 7 months ago

While this actor system is very compelling from my first forays into implementing it, I think it's impractical to make the changes I outlined above right now. While I believe it would ultimately be less code, it would nearly touch every file in the repo.

Instead, I will do solve only the problem at hand by hoisting the FileMachine up to live alongside the Settings and Auth machines, giving Settings access to the currently-opened project and file to allow it to know where to write to.

I will post my findings from exploring the XState V5 actor model into a separate issue, because it will be a substantial refactor if we do it.

franknoirot commented 7 months ago

After discussing a bit more with @Irev-Dev I'm making two more requirements changes:

  1. We're not going to support Part-level settings in this manner. We will likely allow them in a "cargo-script"-like manner down the line.
  2. We'll write these to .toml files, since we were planning on having a project.toml with project info soon anyway. The settings can simply live under a [settings] section.
franknoirot commented 7 months ago

A more fine-grained tasklist:

- [x] rework settings config to easily add to settingsMachine
- [x] persist only changed settings to the appropriate files
- [ ] read persisted changed settings from the appropriate files
- [ ] change settings to modal appearance
- [ ] make all settings show up in command bar
- [ ] make level argument skipped and fixed when not in a project
- [ ] make settings modal UI derived
- [ ] Make commands have optional labels, generate these to look better for settings