-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add config key to set the file creation permissions #537
base: master
Are you sure you want to change the base?
Conversation
lua/oil/config.lua
Outdated
-- Set the default mode to create files (:help oil-file_creation) | ||
create_files_mode = 420, |
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.
We don't expect most users to need to modify this, so let's move it down to below the view_options
.
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.
If we're adding the ability to adjust the mode of the created files, we should do the same for directories. We could use two options for this new_file_mode
and new_dir_mode
.
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.
Since most people are used to thinking about file permissions in octal, the config option should be octal and we should automatically convert it.
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.
ok, sounds good
lua/oil/config.lua
Outdated
@@ -408,6 +412,8 @@ M.setup = function(opts) | |||
if opts.preview and not opts.confirmation then | |||
new_conf.confirmation = vim.tbl_deep_extend("keep", opts.preview, default_config.confirmation) | |||
end | |||
|
|||
M.file_mode = opts.create_files_mode |
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.
Why are we renaming this here?
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 will fix it
doc/oil.txt
Outdated
-------------------------------------------------------------------------------- | ||
FILE CREATION *oil-file_creation* | ||
|
||
|
||
Oil defines `create_files_mode` to set the permission for new files. The mode | ||
follows the Posix numeric standard which use octal notation (base-8), however | ||
the variable must be set in decimal. | ||
|
||
00700 user (file owner) has read, write, and execute permission | ||
00400 user has read permission | ||
00200 user has write permission | ||
00100 user has execute permission | ||
00070 group has read, write, and execute permission | ||
00040 group has read permission | ||
00020 group has write permission | ||
00010 group has execute permission | ||
00007 others have read, write, and execute permission | ||
00004 others have read permission | ||
00002 others have write permission | ||
00001 others have execute permission | ||
(from the open (2) man pages) | ||
|
||
An easy way to provide the right value for `create_files_mode` is using | ||
`tonumber(e [, base])` function with `base = 8`, for example: | ||
>lua | ||
create_files_mode = tonumber("0644", 8) | ||
|
||
The default value is 420 in decimal or 0644 in octal | ||
|
||
|
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 actually don't think we need this section. It's explaining:
- what the octal file permissions mean
- how to convert octal to decimal
- the default value
Point 3 is already documented in the options section. Point 2 is obviated if we accept the permission in octal. And for point 1 I would argue that if you don't know what octal file permissions are, you probably shouldn't be changing this config value and I don't think the oil docs are the correct place to be learning about them.
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.
Ok, then I will remove it
e96ca6f
to
d91e574
Compare
doc/oil.txt
Outdated
-- Set the default mode to create files | ||
new_files_mode = 644, | ||
-- Set the default mode to create folders | ||
new_folder_mode = 755, |
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 am fully aware of how this sounds, and I'm really sorry to be this pedantic, but "folder" is Windows terminology. Typically in Unix contexts we would refer to these as "directories". Elsewhere in this plugin we always refer to these as directories. For consistency sake, could we rename this to new_dir_mode
?
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.
Done
d91e574
to
17ae7b0
Compare
Add config key to set the file creation permissions Rename file_mode to new_file_mode Add new_folder_mode to store the default folder creation mode Remove doc about new_file_mode Fix how the folder and file mode are added to config Rename new_folder_mode to new_dir_mode
17ae7b0
to
e7363e4
Compare
Hey, all changes are done, so it's ready if you want to review it again. Thanks! |
This PR defines a new config key called
create_files_mode
to set the default permission used when a file is created.Currently, Oil uses 644 (420 in decimal) but, at least on Linux most of the tools like (n)vim, touch, or even a simple
> filename
by default uses 664 (436 decimal) so, with this option the user is able to set the default value.