"Suggestion should be comment", but it really shouldn't

Question

I made a suggested edit on a recent answer.

The author of the answer provided a nice solution to the question, but it looks like he didn't have the time to debug it, so his script ended up with some typos, that would prevent someone to use it directly. Namely:

• There was a quoted string that wasn't terminated, so the script couldn't be executed at all
• The script used "$*" instead of "$@", which would cause problems if one of the commands had an argument with spaces in it.
• The script was using the 'exit code of the last command' variable $?, but its value was overwritten by an if statement between issuing the command and using the exit code.

Other that that the answer was very good, innovative, and provided a viable solution to the problem, that hadn't been proposed before in that thread.

So I suggested an edit that fixed all these problems, and also upvoted the answer, which already had been downvoted by someone due to the above typos.

But to my surprise, the edit was rejected, and both reviewers used this justification:

This edit was intended to address the author of the post and makes no sense as an edit. It should have been written as a comment or an answer.

In my opinion this seems nonsensical. I was absolutely NOT addressing the author of the post. I am pretty sure he understands all these issues, but simply didn't notice them, because he didn't test the script (maybe he was on mobile?). It was intended for all the future viewers, that would try to run the script, and possibly be confused if they had less experience in bash.

In addition, the edit,

• was not appropriate as a comment, since newline characters are not available in comments
• was not appropriate as a new answer. Posting as a new answer would be plagiarism, since the idea behind the script had already be posted, and the new answer would just be typo fixes.

So my question is was the suggested edit rejected in error? Or is there a policy on unix.SE, that slightly flawed answers shouldn't be fixed, and all the future readers should spend time debugging around the typos?

If the reviewers aren't confident enough to evaluate the given answer, shouldn't they skip the reviews, rather than letting scripts with typos stay in answers?

Note that both reviewers seem to be "mass-rejectors", in the sense that they have both rejected 55% - 65% percent of their approved suggested edits.

Answer 1

Your edit was indeed helpful. Your changes were absolutely improving the answer and, as you said, it wouldn't even run without the missing quote. Most importantly, you fixed the (( failed )) || return which had to be (( failed )) && return for the script to work.

When looking over your edit, I was confused by your change of

if ! "$*" 
then
  failed=$?
  echo "failed($failed): $*  >&2
fi

To this:

"$@" 
ec=$?
if (( ec ))
then
  failed=$ec
  echo "failed($failed): $*" >&2
fi

I hadn't noticed that the value of $failed was being printed and I had thought that you believed the if would fail unless you rewrote it to use the stored value of $?. On first reading, it looked like you had thought that if command; then wouldn't work and you needed command; ec=$?; if (( $? )); then. Maybe that confused the reviewers as well?

Or maybe they just weren't paying close enough attention, we all make mistakes and it's especially easy when reviewing multiple posts. In any case, you are quite right, that edit should have been approved and there is no policy about leaving imperfect answers alone. On the contrary, we want these kinds of edits, so I have now approved your original suggestion. Thanks for bringing this up, thank you for the edit, sorry it turned out this way and keep up the good work!

Answer 2

1. I agree that the first version of the answer was severely flawed — I’m the one who down voted.
2. I agree that it’s OK for edits to fix technical errors in answers.  Reviewers often reject such edits with the “clearly conflicts with author's intent” reason.  It seems to me that this argument says that the author intended to post a technically flawed answer, which is not nice.
   • It might occasionally be OK to fix technical errors in a question, but you have to be very careful not to invalidate the question by removing the problem that it is asking about.
3. I’m less supportive of suggested edits that make technical changes, because (IMHO) the reviewers are obligated to verify the technical correctness of the edit in order to adjudicate it.
   • On the other hand, what’s the big deal?  There’s a “Skip” button.  If you (a reviewer) can’t determine whether a suggested edit is correct, you should skip it.  This is really true for all kinds of suggested edits.  If an edit suggests punctuation changes, and you don’t know the rules for punctuation in English, you should skip the review.

4. A moderator on Super User has stated that suggested edits should be rejected if the comment is wrong.  I’m inclined to agree.  And your edit comment wasn’t very good.

   • You said “Use "$@" instead of "$*" incase argument has spaces”.

     That’s not the reason.  The reason is that "$*" fails if there are two or more arguments.  For example,

     try ls -ld
     

     gets

     -bash: ls -ld: command not found
     

     P.S. “in case”, in the context of your comment, is two words.  Some dictionaries recognize “incase” as an alternative spelling for “encase”; i.e., to enclose in a case.

   • You said “get exit status immediately, otherwise it will by overwritten by the if's exit status”.

     That’s not the reason.  Given

     if statement1
     then
         statement101
         statement102
         statement103
             ︙
     else
         statement201
         statement202
         statement203
             ︙
     fi

     statement101 and statement201 both “see” (in $?) the exit status from statement1.  Of course, statement101 always sees $? = 0.  The problem is that ! inverts the exit status, so, if the command fails, ! "$@" sets $? to 0.

   • You said “reverse login in first failed test”.

     I guess that this (maybe) refers to changing || to &&, but I don’t quite understand what it means.

     Oh, wait — did you mean “logging in the first failed test”?