Bug with indirect buffers

[walseb]

Indirect buffers inherit all text properties from their base buffer, and so calling jit-lock-fontify-now inside the indirect buffer causes bugs where fortification stops working in the indirect buffer.

I think the text properties that jinx creates itself should be created in the indirect buffer however, in case the base buffer has a different dictionary for instance.

I believe the solution is to wrap jit-lock-fontify-now in (with-current-buffer (or (buffer-base-buffer) (current-buffer)). I'm testing out this change and will submit a draft PR with this change soon.

[minad]

Thanks. Please let me know if your proposed fix works sufficiently well.

[walseb]

Thanks for the fix, but I believe there are more locations where font lock is being called. In my config I add that snippet in 3 locations I believe. I will post it here soon.

[walseb]

Here are a few snippets:

(defun jinx--force-check-region (start end)
    "Enforce spell-check of region between START and END."
    ;; Changed line
    (with-delayed-message (1 "Fontifying...")
      (with-current-buffer (or (buffer-base-buffer) (current-buffer))
        (jit-lock-fontify-now)))
    (with-delayed-message (1 "Checking...")
      (jinx--check-region start end)))

  (defun jinx--cleanup ()
    "Cleanup all overlays and trigger fontification."
    (with-silent-modifications
      (save-restriction
        (widen)
        (jinx--delete-overlays (point-min) (point-max))
        (remove-list-of-text-properties (point-min) (point-max) '(jinx--pending))
        ;; Changed line
        (with-current-buffer (or (buffer-base-buffer) (current-buffer))
          (jit-lock-refontify)))))

  (defun jinx-correct (&optional all)
    "Correct nearest misspelled word.
If prefix argument ALL non-nil correct all misspellings."
    (interactive "*P")
    (unless jinx-mode (jinx-mode 1))
    (cl-letf (((symbol-function #'jinx--timer-handler) #'ignore) ;; Inhibit
              (old-point (and (not all) (point-marker))))
      (unwind-protect
          (let* ((overlays
                  (if all
                      (progn
                        (jinx--force-check-region (point-min) (point-max))
                        (or (jinx--get-overlays (point-min) (point-max))
                            (user-error "No misspellings in whole buffer")))
                    (or (jinx--get-overlays (window-start) (window-end) 'visible)
                        (progn
                          (jinx--force-check-region (window-start) (window-end))
                          (jinx--get-overlays (window-start) (window-end) 'visible))
                        (user-error "No misspelling in visible text"))))
                 (count (length overlays))
                 (idx 0))
            (when all
              (push-mark))
            (while (when-let ((ov (nth idx overlays)))
                     (let* ((deleted (not (overlay-buffer ov)))
                            (skip
                             (catch 'jinx--correct
                               (unless deleted
                                 (jinx--correct
                                  ov all
                                  (and all (format " (%d of %d)" (1+ idx) count)))))))
                       (cond
                        ((integerp skip) (setq idx (mod (+ idx skip) count)))
                        ((or all deleted) (cl-incf idx)))))))
        (when old-point (goto-char old-point))
        ;; Changed line
        (with-current-buffer (or (buffer-base-buffer) (current-buffer))
          (jit-lock-refontify))))))

I can submit a proper PR later if you'd prefer.

[minad]

Thanks. Fixed in https://github.com/minad/jinx/commit/21a299c843e11a5726e6306d9cb34ae2c011e97c.

[walseb]

Thank you! I will try it out.

[walseb]

There appears to still be issues despite these changes. I believe (jit-lock-register #'jinx--mark-pending) also needs to be run in the base buffer, as it appears to trigger an error when run in an indirect buffer.

Although I'm not quite sure what ramifications this change has, I have applied the change locally, and have had no issues for a day. Here's the change:

(with-eval-after-load 'jinx
  (define-minor-mode jinx-mode
    "Enchanted Spell Checker."
    :lighter (:eval (concat " Jinx[" jinx-languages "]"))
    :group 'jinx
    :keymap jinx-mode-map
    (cond
     (jinx-mode
      (jinx--load-module)
      (let ((enable-local-variables :safe))
        (hack-local-variables))
      (jinx--get-org-language)
      (setq jinx--exclude-regexp
            (when-let ((regexps (jinx--mode-list jinx-exclude-regexps)))
              (mapconcat (lambda (r) (format "\\(?:%s\\)" r))
                         regexps "\\|"))
            jinx--include-faces (jinx--mode-list jinx-include-faces)
            jinx--exclude-faces (jinx--mode-list jinx-exclude-faces)
            jinx--camel (or (eq jinx-camel-modes t)
                            (apply #'derived-mode-p jinx-camel-modes))
            jinx--session-words (split-string jinx-local-words))
      (jinx--load-dicts)
      (add-hook 'window-state-change-hook #'jinx--reschedule nil t)
      (add-hook 'window-scroll-functions #'jinx--reschedule nil t)
      (add-hook 'post-command-hook #'jinx--reschedule nil t)
      ;; Changed line
      (with-current-buffer (or (buffer-base-buffer) (current-buffer))
        (jit-lock-register #'jinx--mark-pending)))
     (t
      (mapc #'kill-local-variable '(jinx--exclude-regexp jinx--include-faces
                                                         jinx--exclude-faces jinx--camel
                                                         jinx--dicts jinx--syntax-table
                                                         jinx--session-words))
      (remove-hook 'window-state-change-hook #'jinx--reschedule t)
      (remove-hook 'window-scroll-functions #'jinx--reschedule t)
      (remove-hook 'post-command-hook #'jinx--reschedule t)

      ;; Changed line
      (with-current-buffer (or (buffer-base-buffer) (current-buffer))
        (jit-lock-unregister #'jinx--mark-pending))
      (jinx--cleanup)))))

[minad]

No, enabling jinx-mode in indirect buffers does not seem correct. I pushed https://github.com/minad/jinx/commit/f3a78290090e2754b7a2c4ba0b4b4791e19c2b35 and https://github.com/minad/jinx/commit/3681ae5221c9620b182bd4b3b6bc554f1d0e2d96 which prevent activation.

Can you please give me a recipe to reproduce the issue?

[minad]

@walseb Does it work for you with the recent updates?

[walseb]

I haven't tried it, but I need indirect buffer support for my workflow. I'm using my patch at the moment, which has a few issues. Do you think there is a way indirect buffers could be property supported, given all its issues with local text properties?

[minad]

Indirect buffers should be supported properly. The only condition is that jinx-mode is enabled in the base buffer beforehand. What is different in your use case? Can you create a minimal recipe which demonstrates your use case and shows Jinx issues?

[walseb]

I see, that's excellent, I will have a look in the coming days. Probably the issues I'm having are due to my patch. Thank you!

[walseb]

So far, so good! Thanks for fixing this issue!