-
Notifications
You must be signed in to change notification settings - Fork 4
MacOS Pal Support: Docs and Code Enhancements #31
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,20 @@ package abbr | |
|
|
||
| import ( | ||
| _ "embed" | ||
| "fmt" | ||
| "github.com/scottyeager/pal/inout" | ||
| ) | ||
|
|
||
| //go:embed abbr.fish | ||
| var FishAbbrEmbed string | ||
|
|
||
| func GetFishAbbrScript(abbreviationPrefix string) string { | ||
| return `set -l pal_prefix "` + abbreviationPrefix + `"` + "\n" + FishAbbrEmbed + "\n" | ||
| path, err := inout.GetAbbrFilePath() | ||
| if err != nil { | ||
| // Fallback to Linux default if something goes wrong | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not blindly continue on error. Same for zsh. |
||
| path = fmt.Sprintf("%s", "~/.local/share/pal_helper/expansions.txt") | ||
| } | ||
| return `set -g pal_prefix "` + abbreviationPrefix + `"` + "\n" + | ||
| `set -g pal_abbr_file "` + path + `"` + "\n" + | ||
| FishAbbrEmbed + "\n" | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just use |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,34 @@ import ( | |
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "runtime" | ||
| "strings" | ||
| ) | ||
|
|
||
| const commandFileName = "expansions.txt" | ||
|
|
||
| func GetStoredCommands() (string, error) { | ||
| // GetAbbrFilePath returns the full path to the abbreviations storage file. | ||
| // On macOS it uses: ~/Library/Application Support/pal_helper/expansions.txt | ||
| // On Linux it uses: ~/.local/share/pal_helper/expansions.txt | ||
| func GetAbbrFilePath() (string, error) { | ||
| homeDir, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get home directory: %w", err) | ||
| } | ||
| storagePath := filepath.Join(homeDir, ".local", "share", "pal_helper", commandFileName) | ||
| var storageDir string | ||
| if runtime.GOOS == "darwin" { | ||
| storageDir = filepath.Join(homeDir, "Library", "Application Support", "pal_helper") | ||
| } else { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically on the off chance that someone tries pal on FreeBSD, etc, they will also fall under this branch. As far as I can tell, using the XDG standard on these systems is okay. Maybe we can just mention this possibility in the comment, so it doesn't look like we are using a catchall |
||
| storageDir = filepath.Join(homeDir, ".local", "share", "pal_helper") | ||
| } | ||
| return filepath.Join(storageDir, commandFileName), nil | ||
| } | ||
|
|
||
| func GetStoredCommands() (string, error) { | ||
| storagePath, err := GetAbbrFilePath() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // Ensure directory exists | ||
| storageDir := filepath.Dir(storagePath) | ||
|
|
@@ -41,11 +58,10 @@ func GetStoredCommands() (string, error) { | |
| } | ||
|
|
||
| func StorePrefix0Command(command string) error { | ||
| homeDir, err := os.UserHomeDir() | ||
| storagePath, err := GetAbbrFilePath() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get home directory: %w", err) | ||
| return err | ||
| } | ||
| storagePath := filepath.Join(homeDir, ".local", "share", "pal_helper", commandFileName) | ||
|
|
||
| // Ensure directory exists | ||
| storageDir := filepath.Dir(storagePath) | ||
|
|
@@ -79,11 +95,10 @@ func StorePrefix0Command(command string) error { | |
| } | ||
|
|
||
| func StoreCommands(completion string) error { | ||
| homeDir, err := os.UserHomeDir() | ||
| storagePath, err := GetAbbrFilePath() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get home directory: %w", err) | ||
| return err | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also wrap this error to provide context. Not sure if we are doing that totally consistently, but it seems like a good idea. |
||
| } | ||
| storagePath := filepath.Join(homeDir, ".local", "share", "pal_helper", commandFileName) | ||
|
|
||
| // Ensure directory exists | ||
| storageDir := filepath.Dir(storagePath) | ||
|
|
@@ -116,6 +131,7 @@ func StoreCommands(completion string) error { | |
|
|
||
| // Clean up the old file if it exists. This shouldn't slow us down if the | ||
| // file no longer exists, since stat hits cached data | ||
| homeDir, _ := os.UserHomeDir() | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not be silently dropping the possible err here, I think. Better to keep what we had I think: |
||
| oldPath := filepath.Join(homeDir, ".local", "share", "pal_helper", "completions.txt") | ||
| if _, err := os.Stat(oldPath); !os.IsNotExist(err) { | ||
| // If both exist, remove the old one | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can standardize on
curlhere too (see comment below). But why do we have a differentcurlinvocation in the update command versus here on the README?Also, let's put Linux at the top of the list :)