Inserting "" pair highlights surrounding text with font-lock-string-face

[Hi-Angel]

First of, I'm not sure if this is really the smartparens' problem. However I've occasionally seen similar problems in c++-mode + smartparens too (except I couldn't reproduce them there), and since I have to start somewhere, I figured smartparens would be the closest candidate. Also worth noting, this is very old problem, a few years I think.

Expected behavior

The only text that has font-lock-string-face is the quotes "".

Actual behavior

Whole second line of text glows red, and if you place a caret there and evaluate describe-char, you'll find this text has font-lock-string-face.

Steps to reproduce the problem

1. Make sure you have installed smartparens and csharp-mode
2. Execute in terminal emacs -Q --eval "(progn (package-initialize) (csharp-mode) (smartparens-mode))"
3. In scratch buffer of the appeared Emacs, go to the buffer beginning.
4. Press " (this should make two " symbols to appear).

Environment & version information

• smartparens version: 20190904.1742
• Active major-mode: csharp-mode
• Smartparens strict mode: nil
• Emacs version (M-x emacs-version): GNU Emacs 27.0.50 (build 26, x86_64-pc-linux-gnu, GTK+ Version 3.24.10) of 2019-09-23
• Starterkit/Distribution: Archlinux
• OS: gnu/linux

[Hi-Angel]

So, I figured that most likely this colorization is caused by csharp-mode-syntax-propertize-function.

The most interesting part is that it gets called a lot, like, in a file I tested it in it's hundreds of times! So I went to get a backtrace, and it seems to be called at all times by smartparens:

* csharp-mode-syntax-propertize-function(820 1365)
  syntax-propertize(821)
  syntax-ppss(821)
  sp--syntax-ppss(nil)
  sp-point-in-string(nil)
  sp-point-in-string-or-comment()
  sp--get-handler-context(:post-handlers)
  sp--run-hook-with-args("\"" :post-handlers insert)
  sp-insert-pair()
  sp--post-self-insert-hook-handler()
  self-insert-command(1 34)
  funcall-interactively(self-insert-command 1 34)
  call-interactively(self-insert-command nil nil)
  command-execute(self-insert-command)

Any ideas why smartparens would do this?

[Fuco1]

We are trying to get the syntax context at a particular point to detect if the point is inside a string or not. This is done by syntax-ppss which might do "whatever" the major mode makes it do. SP caches where it can but I don't think this is fixable on our side.

[Hi-Angel]

I see, thanks! As I figure, smartparens does it to figure out whether it wants to insert " or \", right? But as I see after playing a bit in debugger, smartparens does it after the " character gets inserted. Even disregarding the current issue, this seems like a waste since in the end smartparens wants to insert two quotes "" or even the \"\" pair. Could smartparens call the syntax-ppss before the character gets inserted, and replace it with the text it wants?

[Fuco1]

That's one use-case yes. But there are also other rules where doing things inside strings acts differently. If you have the strict mode, wrapping with string quotes or other delimiters also behaves differently, such that you don't do [foo | bar ] region ends HERE ... and turn it into invalid code: [foo " bar ] brace is now part of string " ....

Whether the check is done before or after seems to be like the same deal only you switch the context. We do it after because the entire mechanism is based on post-insert hooks.

[Hi-Angel]

Whether the check is done before or after seems to be like the same deal only you switch the context

Unfortunately it's not. Doing it after is way more CPU-intensive operation than before. Because right after a user inserted a single quote, a lot of text needs to be colorized by the current mode. And then you insert the second quote, and this needs to be done again.

Whereas when it happens before, colorization covers just 2 characters, the "".

[Fuco1]

I don't think this is how font-locking works, it certainly shouldn't do a redraw in the middle of the input loop. And we block the command until everything is inserted.

I might be wrong on this one though.

But it is infeasible now to change this.