Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't launch superfile with a read-only config file #299

Closed
n1yn opened this issue Jul 23, 2024 · 5 comments
Closed

Can't launch superfile with a read-only config file #299

n1yn opened this issue Jul 23, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@n1yn
Copy link

n1yn commented Jul 23, 2024

Description:
When running superfile in the terminal there's an error message saying Error writing config file: open /home/user/.config/superfile/config.toml: read-only file system. I use home-manager to manage my dotfiles, including superfile. This creates a symlink from the nix store to the config file location (read-only). To my understanding reading the config file should not require it to be writable.

Steps to reproduce the behavior:

  1. Tell home-manager to create the configuration file at the specified path
  2. Home-manager creates a symlink from there to the nix store
  3. Starting superfile there's the issue described above

Expected behavior:
Launching normally without requiring to write to the config file on start

System information:

  • OS: NixOS
  • OS Version: 24.05
  • Nix Version: 2.18.4
  • Superfile Version: 1.1.4

Thanks for reading through this and kudos for the amazing file manager

@n1yn n1yn added the bug Something isn't working label Jul 23, 2024
@MrPandir
Copy link

I don't think it's a bug. Since the config is only written to if the required parameters are missing, this is very useful when upgrading.

I think it is better to add an environment variable SPF_STRICT_CONFIG, if it is set to 0 or false, superfile should ignore this error and use default values for missing parameters.

@yorukot
Copy link
Owner

yorukot commented Jul 27, 2024

As @MrPandir said, the main purpose is to write so that when the version update/setting file is missing, it can be automatically supplemented without causing errors.

@Buttars
Copy link
Contributor

Buttars commented Aug 7, 2024

This feature is very inconvenient for declarative OS users. Why not put the fix behavior behind a flag or output what fields are missing so users can have some feedback on what options are missing.

Additionally, imho writing to someones dotfiles without prompting them seems like a bad idea in general.

@Buttars
Copy link
Contributor

Buttars commented Aug 10, 2024

This issue involves several problems and features that need to be addressed:

  1. Handling Incomplete Configurations: How should Superfile manage an incomplete configuration?
  2. User Feedback: How should the user be notified of missing entries?
  3. Default Values: Is it necessary to have all options present in the config to run Superfile, or can defaults be assumed for missing fields?
  4. Hotkeys Configuration: How should the hotkeys configuration be corrected when there are missing fields (automatically, an action, or a flag)

My pull request (PR) introduces a flag that writes default values to any missing entries in the hotkeys configuration instead of doing it every time Superfile is launched. Additionally, it logs each missing entry whenever a user runs Superfile. However, I’m not entirely satisfied with this approach. It was the minimum change required to maintain the ability to write defaults to the config while avoiding automatic modifications at each launch and adding feedback. This breaks automatic config updates when Superfile updates. I don't think this will suffice.

I propose two new actions: validate-hotkeys and fix-hotkeys, which would handle validation and correction respectively. I suggest that defaults should be automatically assumed for any missing entries within the hotkeys config, as this is likely what users would expect.

As far as upgrades are concerned, the upgrade feature can call fix-hotkeys to add the missing properties automatically and a config entry can be added to disable the functionality if desired resolving the immutable OS problem. If the primary function of automatically writing the defaults to the config is to gracefully handle updates it's reasonable to say that writing to the config should be done explicitly during an update and not after every launch. This approach would also give people who don't use the built in update approach the ability to update their configs.

@yorukot
Copy link
Owner

yorukot commented Aug 18, 2024

@Buttars
This is not the best way to do.But I think it's good enough for now.
I would close this. feel free to reopen if you have any new good idea.
And thank you so much for your contribution!

@yorukot yorukot closed this as completed Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants