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.

Sign up for GitHub

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

Jump to bottom

Support relative path for snippet's filename attribute #322

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Support relative path for snippet's filename attribute #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
58ecbb3
doc: missing code block wrappers
csessh Oct 11, 2024
0c724d4
feat: support relative paths for snippet's filename attribute
csessh Oct 11, 2024
48941bc
bug: fix pet new causing panic when SnippetFile attribute is missing
csessh Oct 11, 2024
67985aa
fixed a typo
csessh Oct 11, 2024
8c6671d
fixed another typo
csessh Oct 12, 2024
e87166e
Rewrite ExpandPath to utilise builtin os.UserHomeDir()
csessh Oct 16, 2024
78f9832
Resolve merge conflicts
csessh Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ All `toml` files will be scraped and found snippets will be added.

Example1: single directory

```toml
[GHEGist]
base_url = "" # GHE base URL
upload_url = "" # GHE upload URL (often the same as the base URL)
Expand All @@ -275,6 +276,7 @@ Example1: single directory
gist_id = "" # Gist ID
public = false # public or priate
auto_sync = false # sync automatically when editing snippets
```

```
$ pet configure
Expand Down
2 changes: 1 addition & 1 deletion cmd/edit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func edit(cmd *cobra.Command, args []string) (err error) {
}

if snippetFile == "" {
return errors.New("No sippet file seleted")
return errors.New("No snippet file seleted")
}

// file content before editing
Expand Down
12 changes: 9 additions & 3 deletions cmd/new.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,16 @@ func createAndEditSnippet(newSnippet snippet.SnippetInfo, snippets snippet.Snipp
}

func countSnippetLines() int {
// Count lines in snippet file
f, err := os.Open(config.Conf.General.SnippetFile)
filepath, err := config.ExpandPath(config.Conf.General.SnippetFile)
if err != nil {
return 0
}
Comment on lines +169 to +172
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #321, I made an assumption that if SnippetFile is omitted, it should simply return 0 for line count instead of panicking.

Perhaps whoever understands multiple directories better could correct me here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm yes let me look into this further.

I'm concerned about having a relative snippet file with directories, not sure how that would be handled.
That's even a concern in the current version of pet where you might have an absolute snippet file path as well as directories. Might not be an issue, just have to look into it and possibly document it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having both SnippetDirs and SnippetFile configurable can be rather confusing due to the lack of signs to indicate which one of those two options should take priority.

I'd propose something like this:

SnippetFiles = ["~/.config/pet/snippet-1.toml", "/var/tmp/pet/snippet-2.toml" ]

This consolidates the option of managing multiple directories under one single configurable item.
If you want to keep everything under a single file, specify a single snippet file.
If you want to manage multiple files, in various directories, specify multiple paths.
When save/edit, a prompt to choose which file to save/edit is simple enough.

Of course, I image it'd be a breaking change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RamiAwar I could take a jab at this approach later this week if you think it's worth considering

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csessh sorry I still didn't look into this more, I was actually typing up a reply at some point but lost the tab 😓

I think it's not a problem and there's no work necessary on your end, but I want to better document this. Will respond in a few days time!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to add some more context here - will add it to the docs later as well. You don't have to act on this, just FYI, going through this to review your PR properly.

Having the main snippet file is mandatory.

If snippet directories are also configured, then the snippet Load method will load snippets from the main file + all files inside the snippet directory!

What does this mean for:
1- Edits: How do we edit a snippet if it can be part of any of several files? -> We make the user select the snippet they want to edit first, which tells us which file we should open for editing.

This is still not amazing UX because we send them to the beginning of the file and not the correct snippet line - this should be fixed. (should add a test for this, making sure we jump to the right line + fix it)

2- Search: We just search across all snippets from all files.

More fundamental operations:

3- Load: Loads snippets from the core file (mandatory) + all the files in all directories specified into one big list. Nice, expected.

4- Save: Given a slice of SnippetInfo, this loops over every snippet and checks its filename. This logic doesn't look right to me - we're just overwriting the snippetFile as we loop through? If it's empty, we're setting it to the FIRST snippet directory + a custom file name (???).

I think here it was meant to be snippet.Filename maybe. This definitely seems like a bug, consistent with what you reported in that issue #321 I think.

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it makes sense for the sync to only sync the main snippet file and ignore files in the directories?

I want to try and avoid having to sync multiple files and keep track of each. Directories are nice, but I want to keep it a 'secondary' feature until I iron out the kinks. There are quite some bugs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#323 fixes some of these

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be fine if it's communicated clearly in the tool's documentation that only main snippet file is sync'd.
I, personally, don't use directories. sync or not.


f, err := os.Open(filepath)
if err != nil {
panic("Error reading snippet file")
}

lineCount, err := CountLines(f)
if err != nil {
panic("Error counting lines in snippet file")
Expand Down Expand Up @@ -218,10 +223,10 @@ func _new(in io.ReadCloser, out io.Writer, args []string) (err error) {
}

return createAndEditSnippet(newSnippet, snippets, lineCount+3)

} else {
command, err = scan(color.HiYellowString("Command> "), out, in, false)
}

if err != nil {
return err
}
Expand Down Expand Up @@ -258,6 +263,7 @@ func _new(in io.ReadCloser, out io.Writer, args []string) (err error) {
Command: command,
Tag: tags,
}

snippets.Snippets = append(snippets.Snippets, newSnippet)
if err = snippets.Save(); err != nil {
return err
Expand Down
44 changes: 33 additions & 11 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os/exec"
"path/filepath"
"runtime"
"strings"

"github.com/pelletier/go-toml"
"github.com/pkg/errors"
Expand Down Expand Up @@ -98,9 +99,12 @@ func (cfg *Config) Load(file string) error {
return err
}
var snippetdirs []string
cfg.General.SnippetFile = expandPath(cfg.General.SnippetFile)
RamiAwar marked this conversation as resolved.
Show resolved Hide resolved
for _, dir := range cfg.General.SnippetDirs {
snippetdirs = append(snippetdirs, expandPath(dir)) // note the = instead of :=
if !strings.HasSuffix(dir, "/") {
dir = dir + "/"
}

snippetdirs = append(snippetdirs, dir) // note the = instead of :=
}
cfg.General.SnippetDirs = snippetdirs
return nil
Expand All @@ -109,6 +113,7 @@ func (cfg *Config) Load(file string) error {
if !os.IsNotExist(err) {
return err
}

f, err := os.Create(file)
if err != nil {
return err
Expand All @@ -118,10 +123,16 @@ func (cfg *Config) Load(file string) error {
if err != nil {
return errors.Wrap(err, "Failed to get the default config directory")
}

cfg.General.SnippetFile = filepath.Join(dir, "snippet.toml")
_, err = os.Create(cfg.General.SnippetFile)
file_path, err := ExpandPath(cfg.General.SnippetFile)
if err != nil {
return errors.Wrap(err, "Failed to create a config file")
return errors.Wrap(err, "SnippetFile path is invalid: %v")
}

_, err = os.Create(file_path)
if err != nil {
return errors.Wrap(err, "Failed to create a snippet file")
}

cfg.General.Editor = os.Getenv("EDITOR")
Expand All @@ -132,6 +143,7 @@ func (cfg *Config) Load(file string) error {
cfg.General.Editor = "vim"
}
}

cfg.General.Column = 40
cfg.General.SelectCmd = "fzf --ansi --layout=reverse --border --height=90% --pointer=* --cycle --prompt=Snippets:"
cfg.General.Backend = "gist"
Expand Down Expand Up @@ -165,15 +177,25 @@ func GetDefaultConfigDir() (dir string, err error) {
return dir, nil
}

func expandPath(s string) string {
if len(s) >= 2 && s[0] == '~' && os.IsPathSeparator(s[1]) {
if runtime.GOOS == "windows" {
s = filepath.Join(os.Getenv("USERPROFILE"), s[2:])
} else {
s = filepath.Join(os.Getenv("HOME"), s[2:])
// Given a path to either a file or directory, returns its absolute path format.
// ExpandPath resolves "~/" prefix in a given system path.
// Raise error if path is an empty string as it
func ExpandPath(path string) (string, error) {
if path == "" {
error := errors.New("path to file/directory is not set.")
return path, error
}

if strings.HasPrefix(path, "~/") {
homedir, err := os.UserHomeDir()
if err != nil {
return path, err
}

return filepath.Join(homedir, path[2:]), nil
}
return os.Expand(s, os.Getenv)

return path, nil
}

func isCommandAvailable(name string) bool {
Expand Down
40 changes: 27 additions & 13 deletions snippet/snippet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,45 +30,53 @@ func (snippets *Snippets) Load(includeDirs bool) error {
// Create a list of snippet files to load snippets from
var snippetFiles []string

// Load snippets from the main snippet file
// Raise an error if the file is not found / not configured
snippetFile := config.Conf.General.SnippetFile
if snippetFile != "" {
snippetFile, err := config.ExpandPath(config.Conf.General.SnippetFile)
if err == nil {
if _, err := os.Stat(snippetFile); err == nil {
snippetFiles = append(snippetFiles, snippetFile)
snippetFiles = append(snippetFiles, config.Conf.General.SnippetFile)
} else if !os.IsNotExist(err) {
return fmt.Errorf("failed to load snippet file. %v", err)
} else {
return fmt.Errorf(
`snippet file not found. %s
Please run 'pet configure' and provide a correct file path, or remove this
if you only want to provide snippetdirs instead`,
snippetFile,
config.Conf.General.SnippetFile,
)
}
}

// Optionally load snippets from snippet directories
if includeDirs {
for _, dir := range config.Conf.General.SnippetDirs {
for _, snippetDir := range config.Conf.General.SnippetDirs {
dir, err := config.ExpandPath(snippetDir)
if err != nil {
return fmt.Errorf("snippet directory not found. %s", snippetDir)
}

if _, err := os.Stat(dir); err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("snippet directory not found. %s", dir)
return fmt.Errorf("snippet directory not found. %s", snippetDir)
}
return fmt.Errorf("failed to load snippet directory. %v", err)
snippetFiles = append(snippetFiles, getFiles(dir)...)
}

snippetFiles = append(snippetFiles, getFiles(dir)...)
}
}

// Read files and load snippets
for _, file := range snippetFiles {
tmp := Snippets{}
f, err := os.ReadFile(file)
file_path, err := config.ExpandPath(file)
if err != nil {
return fmt.Errorf("failed to load snippet file. %v", err)
}

f, err := os.ReadFile(file_path)
if err != nil {
return fmt.Errorf("failed to load snippet file. %v", err)
}

tmp := Snippets{}
err = toml.Unmarshal(f, &tmp)
if err != nil {
return fmt.Errorf("failed to parse snippet file. %v", err)
Expand Down Expand Up @@ -98,7 +106,13 @@ func (snippets *Snippets) Save() error {
newSnippets.Snippets = append(newSnippets.Snippets, snippet)
}
}
f, err := os.Create(snippetFile)

file_path, err := config.ExpandPath(snippetFile)
if err != nil {
return fmt.Errorf("failed to save snippet file. err: %s", err)
}

f, err := os.Create(file_path)
if err != nil {
return fmt.Errorf("failed to save snippet file. err: %s", err)
}
Expand Down
12 changes: 9 additions & 3 deletions sync/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,22 +100,28 @@ func upload(client Client) (err error) {
// download downloads snippets from the remote repository
// and saves them to the main snippet file - directories ignored
func download(content string) error {
snippetFile := config.Conf.General.SnippetFile

var snippets snippet.Snippets
if err := snippets.Load(false); err != nil {
return err
}

body, err := snippets.ToString()
if err != nil {
return err
}

if content == body {
// no need to download
fmt.Println("Already up-to-date")
return nil
}

fmt.Println("Download success")
return os.WriteFile(snippetFile, []byte(content), os.ModePerm)

file_path, err := config.ExpandPath(config.Conf.General.SnippetFile)
if err != nil {
return err
}

return os.WriteFile(file_path, []byte(content), os.ModePerm)
}