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

chmod: Add recursive 755/644 chmod example #1651

Merged
merged 3 commits into from
Nov 30, 2017
Merged

chmod: Add recursive 755/644 chmod example #1651

merged 3 commits into from
Nov 30, 2017

Conversation

mathomp4
Copy link
Contributor

@mathomp4 mathomp4 commented Nov 24, 2017

Might not be approved, but this is the classic 755 on all directories, 644 on all files in a tree a la: https://stackoverflow.com/questions/18817744/change-all-files-and-folders-permissions-of-a-directory-to-644-755

As I just discovered tldr, I had to try my hand at updating a page with a command I have to Google every single time.


  • The page (if new), does not already exist in the repo.

  • The page (if new), has been added to the correct platform folder:
    common/ if it's common to all platforms, linux/ if it's Linux-specific, and so on.

  • The page has 8 or fewer examples.

  • The PR is appropriately titled:
    <command name>: add page for new pages, or <command name>: <description of changes> for pages being edited

  • The page follows the contributing guidelines

Might not be approved, but this is the classic 755 on all directories, 644 on all files in a tree a la: https://stackoverflow.com/questions/18817744/change-all-files-and-folders-permissions-of-a-directory-to-644-755

As I just discovered `tldr`, I had to try my hand at changing a command
@CLAassistant
Copy link

CLAassistant commented Nov 24, 2017

CLA assistant check
All committers have signed the CLA.

@mathomp4 mathomp4 changed the title Update chmod.md: Add recursive chmod chmod: Add recursive 755/644 chmod example Nov 24, 2017
Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Thanks for the addition, @mathomp4! I didn't realise that you could separate multiple permissions operations with a comma like that. I'll have to remember that!

Anyway, this looks ok to me! Once we get a second opinion here we'll get to merging.

@@ -21,3 +21,7 @@
- Give [o]thers (not in the file owner's group) the same rights as the group:

`chmod o=g {{file}}`

- Change permissions [-R]ecursively giving the [u]ser [r]ead, [w]rite, and e[X]ecute rights, and give [g]roup and [o]thers [r]ead and e[X]ecute rights but not [-w]rite rights:
Copy link
Member

Choose a reason for hiding this comment

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

I like what you've you've done here to aid in the understanding of the command! Cool 😃


- Change permissions [-R]ecursively giving the [u]ser [r]ead, [w]rite, and e[X]ecute rights, and give [g]roup and [o]thers [r]ead and e[X]ecute rights but not [-w]rite rights:

`chmod -R u+rwX,go+rX,go-w {{directory}}`
Copy link
Member

Choose a reason for hiding this comment

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

Can we just keep this example to "user" and remove the "group" and "others" ? I fear the description is getting a bit too long. And the "group" and "others" are already explained in earlier examples. Since, this example just introduces the -R option, I think the users can build up the go options themselves.

And also, I think we can use x instead of X. The man page does say that X is for execute/search only if the file is a directory or already has execute permission for some user (X), but I did not find any difference with x in my tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, capital X is usually preferred because x can often do things in a way you wouldn't suspect:

Indeed, looking at that last link, this:

chmod -R u=rwX,g=rX,o=rX {{directory}}

might be a better command example. With this you are expressly saying u gets rwX and g and o get rX.

Copy link
Member

Choose a reason for hiding this comment

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

I see. My only concern is earlier we use x but in this example, we use X without really explaining the difference. And if we attempt to explain the difference, the description will become huge.

Therefore, in the spirit of simplicity, I wanted to go with x.

I am open to any other suggestions you have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Well, I can just close the request in the sake of non-confusion.

Do you know if there is a way to provide a "custom page path" to tldr? Such that I could add my pages/common/chmod.md so my tldr sees it, but there isn't a need to get it in the official tree?

Copy link
Member

Choose a reason for hiding this comment

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

We don't need to close it. I would still want the -R option to be added.

Do you know if there is a way to provide a "custom page path" to tldr?

If you use the node client, yes there is. https://github.com/tldr-pages/tldr-node-client#configuration. See last section. Though its not exactly what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, if you want to not use X we should avoid it completely. For a recursive use perhaps we could add chmod -R g+w,o+w:

$ tree -p Dir1/
Dir1/
├── [-rw-r--r--]  file1
└── [-rw-r--r--]  file2

0 directories, 2 files
$ chmod -R g+w,o+w Dir1/
$ tree -p Dir1/
Dir1/
├── [-rw-rw-rw-]  file1
└── [-rw-rw-rw-]  file2

0 directories, 2 files

That adds write to both group and other.

@agnivade agnivade added the page edit Changes to an existing page(s). label Nov 27, 2017
@agnivade
Copy link
Member

@sbrl - Can you take a look at this PR again ? I had some concerns due to which the PR has changed a bit. Please look at my comments here - #1651 (comment). Let me know if you are good with these.

@sbrl
Copy link
Member

sbrl commented Nov 30, 2017

Yeah sure, @agnivade.

It still looks ok - much simpler and easy to understand :D

@sbrl sbrl merged commit f58ace6 into tldr-pages:master Nov 30, 2017
@sbrl
Copy link
Member

sbrl commented Nov 30, 2017

Thanks, @mathomp4!

@sbrl sbrl mentioned this pull request Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
page edit Changes to an existing page(s).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants