Setting `,` syntax to whitespace shouldn't happen inside docstrings

[jaor]

Expected behavior

When delete-trailing-whitespace mode is enabled (or the function called by hand),
trailing commas at the end of a docstring line shouldn't be deleted.

Actual behavior

Commas at the end of a docstring line are deleted.

Steps to reproduce the problem

• Open foo.clj
• Type

   (def foo
       "A docstring with more than a line,
        and a comma at the end of the previous one."
      42)

• Extend the region to the full file, e.g. C-x h
• M-x delete-trailing-whitespace

The command at the end of "than a line," is deleted.

Environment & Version information

emacs 27 pre-release

clojure-mode version

clojure-mode (version 5.12.0-snapshot)

Emacs version

27 pre-release

Operating system

Debian sid

[bbatsov]

@yuhan0 Can you take a look at this problem?

[yuhan0]

There doesn't seem to be an immediate fix for this besides reverting the commit.

Sorry I didn't anticipate something like this, I've been using this change for months without any side effects, but I use ws-butler package for whitespace cleanup, which uses a hardcoded " \t" regexp instead of syntax.

This looks like a serious enough bug that I think it should be reverted first.

[yuhan0]

So it turns out syntax-propertize-rules (emacs 24+) can be used to set commas to whitespace depending on the context:

(defun clojure-syntax-propertize-function (start end)
  (goto-char start)
  (while (and (< (point) end)
              (re-search-forward ",+" end t))
    (let ((ppss (syntax-ppss)))
      (when (or (nth 3 ppss) (nth 4 ppss))
        (put-text-property (match-beginning 0) (match-end 0)
                           'syntax-table (string-to-syntax "."))))))

...

(setq-local syntax-propertize-function #'clojure-syntax-propertize-function)

But this means that delete-trailing-whitespace will still remove commas at the end of lines like

{:a 1, :b 2,
 :c 3, :d 4}

which I'm not sure is the expected behaviour either.

[yuhan0]

Yeah, you wouldn't want trailing commas to be removed in cases like:

(-> m
  (assoc ,,,
    :foo 123))

I think the issue is that commas are semantically meaningful to human readers, butfunctions like delete-trailing-whitespace operate on the assumption that syntactic whitespace can always be removed without concern.

I'll open a PR to revert the change, as the benefits probably don't outweigh the possibility of bugs like this.

[mattiuusitalo]

This bug mangled some json strings I had in a clojure source file. For example:

(def foo 
    "\"foo\": 1,
     \"bar\": 2}")

Would be destroyed. I'm not sure if the fix has already been released, but something like this could be added to the test cases.

[yuhan0]

I can confirm this was fixed by #562. Will add the test case along with my next PR.