Hello @GLobyNew, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses issue #3075, allowing the use of a trailing slash in cp and mv commands. The changes modify the directMove functions in internal/store/leaf/move.go and internal/store/root/move.go to handle trailing slashes in the destination path. Specifically, if the destination path ends with a slash, the code now appends the base name of the source file to the destination path, effectively creating a new directory and moving/copying the source file into it. Additionally, the store prefix removal logic in internal/store/root/move.go was adjusted to avoid removing the prefix from the destination path, as it might represent a new folder.

Highlights

• Bug Fix: Fixes issue #3075, enabling the use of trailing slashes with cp and mv commands.
• Trailing Slash Handling: Implements logic to interpret a trailing slash in the destination path as a request to create a new directory and move/copy the source file into it.
• Store Prefix Handling: Adjusts store prefix removal logic in internal/store/root/move.go to correctly handle destination paths that may represent new folders.

Changelog

• internal/store/leaf/move.go
  • Added a check for a trailing slash in the destination path (to).
  • If a trailing slash is present, the base name of the source file is appended to the destination path.
  • This ensures that the file is moved/copied into a new directory with the same name.
• internal/store/root/move.go
  • Modified the directMove function to handle trailing slashes in the destination path.
  • The store prefix is no longer removed from the destination path (to) to allow for creating new folders.
  • Adjusted the getStore call to only remove the store prefix from the source path (from).

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

A trailing slash, a simple sign,
To move a file, a new design.
In gopass code, the fix takes hold,
A folder made, a story told.

Code Review

The pull request introduces a fix to allow the use of a trailing slash for the cp and mv commands. This change modifies the directMove functions in both the leaf and root store implementations to handle destinations with trailing slashes, treating them as requests to create a new folder and move/copy the source file into it. The changes also adjust how the destination path is handled in the root store to accommodate this new behavior. Overall, the changes seem reasonable, but there are a few areas that could be improved for clarity and robustness.

Summary of Findings

• Inconsistent destination path handling: The root store's directMove function modifies how the destination path is processed, potentially leading to inconsistencies in how paths are handled across different store types. This could introduce unexpected behavior or bugs in other parts of the application.
• Lack of input validation: The code does not explicitly validate the destination path to ensure it's a valid directory name. This could lead to issues if the destination contains invalid characters or is otherwise malformed.
• Potential for directory creation failure: The code assumes that the destination directory will be created successfully. However, there is no explicit error handling for the case where directory creation fails, which could lead to unexpected behavior or data loss.

Merge Readiness

The pull request addresses a specific issue and includes passing tests, suggesting it resolves the reported problem. However, the potential inconsistencies in destination path handling and the lack of explicit error handling for directory creation failures raise concerns. I recommend addressing these issues before merging. I am unable to approve this pull request, and users should have others review and approve this code before merging.

Thanks a lot. I was missing two key pieces: (a) that it old affects REPL and (b) that we were trying to move to a non-existing folder.