Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
>> Cc: 66546@debbugs.gnu.org
>> Date: Thu, 19 Oct 2023 23:12:59 +0200
>>
>> But probably it would be helpful to shortcut that look-up process, like
>> this:
>
> I don't object, though a more detailed explanation (instead of sending
> people to read the bug discussion) would be better.

I added the reasons why you wanted to keep the call to
`set-file-extended-attributes' in the attached patch.

>> +             ;; If we get an error writing the file which we
>> +             ;; previously made writable, attempt to undo the
>> +             ;; write-access.
>> +             ((and (eq tempsetmodes 'u+w) (not success))
>
> Isn't it easier, safer, and more portable to compare buffer-file-name
> with (nth 2 setmodes) instead?

I wanted to make 100% sure that we execute that first cond-branch if and
only if we previously changed the file mode.  IOW, I feel I cannot
exclude that by some strange configuration

  (equal buffer-file-name (nth 2 setmodes))

could also be true in other cases.

I added more documentation and renamed the u+w symbol to something
looking less Unix-specific.

>> +              (condition-case ()
>> +		  (unless
>> +		      (with-demoted-errors "Error setting file modes: %S"
>> +			(set-file-modes buffer-file-name (car setmodes)))
>> +		    (set-file-extended-attributes buffer-file-name
>> +						  (nth 1 setmodes)))
>> +		(error nil)))
>
> Why do we need condition-case here if we use with-demoted-errors?

We don't need it, I just copied that snippet from function
`basic-save-buffer' without thinking about it.  I changed that.

As mentioned earlier, I can also add unit tests for both issues.  If I
remember correctly, you would prefer these in the same changeset.

Please review the next version of the patch.

Thanks.