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

feat: add support for copying multiple files to clipboard #13

Merged
merged 1 commit into from
Jun 23, 2024
Merged

Conversation

supitsdu
Copy link
Owner

  • Updated main.go to allow specifying multiple file paths
  • Concatenated content from multiple files before copying to clipboard
  • Updated usage message to reflect new functionality

- Updated main.go to allow specifying multiple file paths
- Concatenated content from multiple files before copying to clipboard
- Updated usage message to reflect new functionality
@supitsdu supitsdu added the enhancement New feature requests or enhancements. label Jun 23, 2024
@supitsdu supitsdu merged commit 9ce81fa into main Jun 23, 2024
1 check passed
@supitsdu supitsdu deleted the feat branch June 23, 2024 04:37
if len(flag.Args()) == 1 {
// Read the content from the file path provided
contentStr, err = readFromFile(flag.Arg(0))
if len(flag.Args()) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

The complexity of the code is increased by the multiple branching if

What about moving these to a function

contentStr, err := parseContent(flag)
if err != nil {
fmt.Println
os.Exitt(1)
}

// …

func parseContent(flag) string, error {
   if directText != {
      return *directText
   }

   // …
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for the suggestion! Refactoring to move the content parsing logic into a new parseContent function does indeed reduce complexity in the main function. I agree with the approach and will address this in a future update, as I'm currently focusing on the installation script. If you’d like to expedite this improvement, please feel free to open a PR.

Copy link
Contributor

@ccoVeille ccoVeille Jun 23, 2024

Choose a reason for hiding this comment

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

I can but not in the next days, so I'm moving my comment to an issue, so it can be addressed later either by me, you, or anyone

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for the suggestion and for understanding. Moving your comment to an issue is a great idea so that it can be tracked and addressed later by anyone available. I appreciate your input and look forward to collaborating on this improvement when possible. If you or anyone else gets a chance to work on it before I do, that would be fantastic.

Copy link
Contributor

Choose a reason for hiding this comment

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

=> #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature requests or enhancements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants