Re: [PATCH] add: add --chmod=+x / --chmod=-x options

Johannes Schindelin [off-list ref] writes:

I wonder, however, whether it would be "cleaner" to simply make this an
OPT_STRING and perform the validation after the option parsing.

Yes, I think I touched on this in my comments in a bit more detail.

Hmm. This change uses up 2 out of 31 available bits. I wonder whether a
better idea would be to extend struct update_callback_data to include a
`force_mode` field, pass a parameter of the same name to
add_files_to_cache() and then handle that in the update_callback().

Maybe.  I am not sure if it is a good idea to do lstat(2) on the
calling side, though.  Assuming it is, your "something like this"
needs to be duplicated for the codepath that adds a new file, which
is separate from the one we see below (i.e. add_files()).

Something like this:

                case DIFF_STATUS_MODIFIED:
-               case DIFF_STATUS_TYPE_CHANGED:
+               case DIFF_STATUS_TYPE_CHANGED: {
+			struct stat st;
+			if (lstat(path, &st))
+				die_errno("unable to stat '%s'", path);
+			if (S_ISREG(&st.st_mode) && data->force_mode)
+				st.st_mode = data->force_mode;
-                       if (add_file_to_index(&the_index, path, data->flags)) {
+                       if (add_to_index(&the_index, path, &st, data->flags)) {
                                if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
                                        die(_("updating files failed"));
                                data->add_errors++;
                        }
                        break;
+		}