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

find: fix two examples with wrong or misleading syntax #15108

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

find: fix two examples with wrong or misleading syntax #15108

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3ecefc
fix wrong or misleading syntax in two find examples
zmajeed Dec 9, 2024
0f299c5
fix wrong or misleading syntax in two find examples
zmajeed Dec 9, 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
4 changes: 2 additions & 2 deletions pages/common/find.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

- Find files matching multiple path/name patterns:

`find {{root_path}} -path '{{**/path/**/*.ext}}' -or -name '{{*pattern*}}'`
`find {{root_path}} -path '{{*/path/*/*.ext}}' -or -name '{{*pattern*}}'`
sebastiaanspeck marked this conversation as resolved.
Show resolved Hide resolved

- Find directories matching a given name, in case-insensitive mode:

Expand All @@ -33,4 +33,4 @@

- Find empty files (0 byte) or directories and delete them verbosely:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Find empty files (0 byte) or directories and delete them verbosely:
- Find empty files or directories and delete them verbosely:

I feel like the parenthese creates an unnecessary gap before the word "or"

Copy link
Author

Choose a reason for hiding this comment

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

Agree - "(0 byte)" is clunky and ugly

Copy link
Author

Choose a reason for hiding this comment

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

I like this better "List and delete empty files and directories"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Find empty files (0 byte) or directories and delete them verbosely:
- Search for either empty files or directories and delete them verbosely:

This maybe?


`find {{root_path}} -type {{f|d}} -empty -delete -print`
`find {{root_path}} -type '{{f,d}}' -empty -delete -print`
Copy link
Member

Choose a reason for hiding this comment

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

This example is not wrong :D I'd rather leave it like this. The | is fine, because you either have a f (for file) or a d for directory there.

Copy link
Author

Choose a reason for hiding this comment

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

Have you tried to run the example?

Copy link
Member

Choose a reason for hiding this comment

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

The | is tldr syntax (similar to {{}}), so users would change the command accordingly to either contain -type f or -type d.

However, I realise that the current form of the command is indeed misleading. I would wait what others have to say on this issue.

Copy link
Author

Choose a reason for hiding this comment

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

No problem - though I think the example text Find empty files (0 byte) or directories and delete them verbosely is pretty specific and doesn't make sense without the corrected syntax

Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the event that made you feel that this command needed to be changed? I want to understand if it was a clarity issue or if it's just misunderstanding for not knowing the syntax.

Copy link
Author

Choose a reason for hiding this comment

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

The command for the example is wrong - the syntax is wrong in two ways - I've fixed it to run and correctly work for the described purpose

Copy link
Collaborator

Choose a reason for hiding this comment

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

image
Works fine for me.

Copy link
Author

Choose a reason for hiding this comment

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

it skips directories

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well yes, the description does say "or". It can be interpretted in two ways I guess. I'll ruminate on this for a while and try to come up with a a better wording. I do think f|d is the way to go, just that the wording needs to be more precise that the user needs to choose.

Loading