cc-tweaked / CC-Tweaked

Just another ComputerCraft fork
https://tweaked.cc
940 stars 211 forks source link

Persistence of shell aliases/completions, settings definitions. #2005

Closed fatboychummy closed 3 weeks ago

fatboychummy commented 3 weeks ago

A common problem with using settings definitions, shell aliases, and completions, is that they do not persist across a computer restart, unless you custom-build your program to write a file to something like /startup/99_definitions.lua. This works, but is (in my opinion) not very ideal, nor intuitive -- most people don't even realize you can make a startup folder! Thus, I decided to draft up this PR.

With this, users can now add all of this information in their installer program and have it registered only once, but persist forever. It will also still work if the definitions are placed in the main program itself (as most people currently do things). However, the user won't have to run the program without arguments (or register a startup file) in order to get completions or settings definitions. Or at the very least, they only have to do it once, instead of once per startup.

Contents

This PR adds/modifies the following methods:

Settings

Additions

Alterations

Shell

Additions

Alterations

BIOS

Additions

Alterations

Startup

Additions

Alterations

.cc Folder Structure

.cc/
| - settings : The settings file
| - settings.def : The settings definitions file
| - aliases/ : Each file contained within is a single alias
    | - sh : Example, would contain "shell"
| - completions/
    | - example.lua : Contains the code for shell completion of `example.lua`

Considerations

As outlined above, the settings are now stored by default in /.cc/settings (and /.cc/settings.def). This could cause some backwards compatibility issues if a program manually alters /.settings assuming that settings.save() will store the data there.

Edit: There are a few more edge cases that were discussed in Discord.

  1. User program saves a CraftOS-specific setting and passes .settings to settings.save instead of relying on the default: Upon restart, the setting will no longer be set, as .settings is no longer the default.
  2. In a similar vein, if the user .save()s without an argument, then .load(".settings")s, this will also fail.
  3. A program may rely on loading from .settings, but the set program would now default to storing in .cc/settings

Honestly, I will probably revert this change, but would like more feedback before I do so!


This is very much a draft PR, I need to write tests still as well as some general cleanup (i.e: variable names, need to use fs.combine in some locations, etc.). I wanted to get feedback on this before I went ahead on that step however. Thoughts on this? Concerns?

SquidDev commented 3 weeks ago

Thanks for the PR! While I understand the motivation, I'm afraid I'm not going to merge this, for a couple of reasons:

I'm definitely open to doing a better job of documenting startup files. There was some work done on this in #1069, but that's stalled. I'm definitely open to someone else having another look at it.