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

added getUniquePath function #106

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mosajjal
Copy link

Added a function to get a unique filename so we don't accidentally replace an existing file

Added a function to get a unique filename so we don't accidentally replace an existing file
cmd/ww/file.go Outdated
// find a suitable filename to receive a file. if the path already exist, append a suffix or increment the existing suffix
func getUniquePath(path string) string {
newpath := path
suffixpattern := regexp.MustCompile(`\.(\d+)$`)
Copy link
Owner

Choose a reason for hiding this comment

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

regexp is not imported

@saljam
Copy link
Owner

saljam commented Nov 19, 2022

i know it's been a while, but thanks for sending this in! i like the feature and i think we should definitely do this.

i have 2 comments on how this is done however:

  • we should be able to do it without a regex. the strings package has enough helpers to make this fairly easy.
  • it would be nice to insert the counter before the extension, if an extension is present.

not specific to this, but a related idea: we should also add a suffix to files in progress, and rename them once they are complete.

@mosajjal
Copy link
Author

hey. yep, makes sense. I think at the time I was looking for MVP and see what you think. I'll push some changes to this before marking it as ready to merge

@mosajjal
Copy link
Author

changed the way it finds a new path and removed regex. added a few test cases below it too, but not sure if this function or the test suite should sit in the main package.

@mosajjal mosajjal requested a review from saljam November 28, 2022 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants