added scanning of paths for flatpak & snap install#76
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Linux-specific discovery for Obsidian’s obsidian.json when installed via Flatpak or Snap so commands like set-default can locate the correct config file.
Changes:
- Updated
config.ObsidianFile()to scan Linux candidate locations (native, Flatpak, Snap) and return the first existing config file path. - Expanded unit tests to cover Linux Flatpak/Snap detection, fallback behavior, and precedence rules.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/config/obsidian_path.go | Adds Linux-aware candidate path scanning to locate an existing obsidian.json. |
| pkg/config/obsidian_path_test.go | Adds Linux-only tests validating Flatpak/Snap discovery and precedence/fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| originalUserConfigDir := config.UserConfigDirectory | ||
| defer func() { config.UserConfigDirectory = originalUserConfigDir }() | ||
|
|
||
| tempDir := t.TempDir() | ||
| flatpakDir := filepath.Join(tempDir, ".var", "app", "md.obsidian.Obsidian", "config", "obsidian") |
There was a problem hiding this comment.
Many of the Linux subtests repeat the same setup (stubbing config.UserConfigDirectory, setting/restoring $HOME, creating config files). Consider extracting small helper(s) to reduce duplication so the tests stay easier to maintain as more install paths/precedence rules are added.
There was a problem hiding this comment.
I was trying to be as unobtrusive as possible and taking on KISS. If this were for 8 or more I would have taken that approach, but this (I'm guessing AI?) comment increases complexity without making a substantive difference. If you prefer, I can take this approach. Just let me know.
There was a problem hiding this comment.
Thanks for checking, lets keep it as it is!
|
@mikesale - thank you for the PR! Could you please take a look at the review |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
I took the suggested change for the source file and have left the test unchanged pending your feedback. |
Added scanning for installs of obsidian on linux by flatpak and snap
Description
changed obsidian_path.go and its test to handle fails on linux to also look in standard locations for flatpak and snap installs.
Motivation and Context
Without this set-default does not work properly.
Checklist:
I have also tested this on via a flatpak install and it works.
n.b. this method could also be used to also add WSL on windows detection. I would add it, but I don't know the correct pattern.