-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
new occ commands to manage system-tags for files #48277
base: master
Are you sure you want to change the base?
new occ commands to manage system-tags for files #48277
Conversation
apps/files/lib/Command/TagAdd.php
Outdated
->setDescription('Add a system-tag to a file or folder') | ||
->addArgument('target', InputArgument::REQUIRED, "file id or path") | ||
->addArgument('tags', InputArgument::REQUIRED, "Name of the tag(s) to add, comma separated") | ||
->addArgument('access', InputArgument::REQUIRED, 'access level of the tag (public, restricted or invisible)'); |
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.
It feels weird that access is required for assigning an existing tag
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.
I think it is required because
- the same tagName can exist with different access level
- I want to work around the missing exif support (Show and edit EXIF Tags photos#226) by automatically creating the tags if it does not exist
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.
I want to work around the missing exif support (nextcloud/photos#226) by automatically creating the tags if it does not exist
We do support EXIF data, they are currently stored with the FilesMetadata API: https://github.com/nextcloud/photos/blob/f70a0486642e166c69bdd45a480288e05ba5faa9/lib/Listener/ExifMetadataProvider.php
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.
please replace EXIF with metadata/xmp...
I'm using the exiftool to extract metadata. The tool is able to extract the XMP information and TagsList is part of XMP, not EXIF
ExifMetadataProvider does indeed store the EXIF part of the metadata, but this only includes things like GPS, Time, size,...
34cde37
to
819385c
Compare
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
819385c
to
c57f2fa
Compare
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.
Looks good
class DeleteAll extends Command { | ||
|
||
public function __construct( | ||
private FileUtils $fileUtils, |
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.
I'm not sure if FileUtils is recommended to be used in apps even if they're in server
@schaarsc Psalm is still failing with an error talking about a wrong argument type |
Resolve nextcloud#32735 Signed-off-by: schaarsc <schaarsc@users.noreply.github.com>
c57f2fa
to
778ae6a
Compare
How do we merge since CI fails because it doesn't want to run on forks? |
Ask a team lead to force-merge |
Summary
adds the following commands to the files app:
files:tag-add <target> <tags> <access>
files:tag-delete <target> <tags> <access>
files:tag-delete-all <target>
TODO
Checklist