Created attachment 337196 [details] [review]
regular-file-function: reword the function name

Created attachment 337231 [details] [review]
regular-file-function: reword the function name

In commit we reword the "not file is directory" to
"file is regular file"

The problem is that there are some files available which contain
"!nautilus_file_is_directory" which not look good in file.

To fix this issue we used the already available function, that is
"nautilus_file_is_regular_file()" which is exactly equal to the
function "!nautilus_file_is_directory()"

Review of attachment 337231 [details] [review]:

I do not think the code reads any better with your changes (and is likely incorrect).

Judging by the diff, more often than not, the code only works with directories, so checking if the file is a regular file does not necessarily have the same effect.

You and Carlos were discussing this, so I propose we wait for his input (maybe I am wrong about something, too), but I just wanted to express my thoughts on this.

::: src/nautilus-file.c
@@ +7796,3 @@
            nautilus_file_can_execute (file) &&
            nautilus_file_is_executable (file) &&
+           nautilus_file_is_regular_file (file);

Quite honestly, I think this is the only place in code, where checking if the file is a regular file makes sense.

Even I feel that I did some mistake and in following commit I really did silly mistake.

::: src/nautilus-properties-window.c
@@ -3435,7 +3435,7 @@ update_permissions (NautilusPropertiesWindow *window,
 
         if (!apply_to_both_folder_and_dir &&
             ((nautilus_file_is_directory (file) && !is_folder) ||
-             (!nautilus_file_is_directory (file) && is_folder)))
+             (nautilus_file_is_regular_file (file) && is_folder)))

These conditions will never satisfy :( I don't know why I wrote this

(In reply to Neha Yadav from comment #4)
> These conditions will never satisfy :( I don't know why I wrote this

They will, do not worry. It goes through the list of files, for which the properties are being displayed, and checks whether the permission change should be applied. You can change permissions for files and folders separately, if you have a mixture of both selected.

Time has forgotten about the code for the properties window and it is quite messy. You did not make the mistake you think you made.

Sorry if I sounded a bit harsh in my review. I try to wear my serious hat when commenting on Bugzilla. :)

Review of attachment 337231 [details] [review]:

Ernestas is right, only that one makes sense to convert to *_is_regular_file

Created attachment 338588 [details] [review]
regular-file-function: reword the function name

In commit we reword the "not file is directory" to
"file is regular file"

The problem is that there is a file available which contain
"!nautilus_file_is_directory" which not look good in file.

To fix this issue we used the different function, that is
"nautilus_file_is_regular_file()" which is exactly equal to the
function "!nautilus_file_is_directory()"

Review of attachment 338588 [details] [review]:

It ended up being extremely short. :|

The only nitpicking I want to do is wrt the commit message:

The subject line should say what file has been touched (in this case, just “file”);
the two functions are not exactly antonyms of each other, so the body needs rewriting (e.g. “now that we have the API, we can check if the file is a regular file where needed and not check if it is not a directory”).

I am very bad in writing commit messages :/

Created attachment 339010 [details] [review]
Add function nautilus_file_is_regular_file

Created attachment 339011 [details] [review]
regular-file-function: reword the function name


The problem is that there is a file available which contain "!nautilus_file_is_directory" which not look good in file.
    
Now we have the API, we can check if the file is a regular file
where needed and not check if it is not a directory.

Review of attachment 339011 [details] [review]:

Excellent, thanks!

Attachment 339011 [details] pushed as 3d50cea - regular-file-function: reword the function name