fxbois / web-mode

web template editing mode for emacs
https://web-mode.org
GNU General Public License v3.0
1.64k stars 260 forks source link

Current column highlight displayed on folded elements #1274

Closed jmckalex closed 1 year ago

jmckalex commented 1 year ago

When web-mode-enable-current-column-highlight is set to t, the highlighted column is still shown even when the element is folded (see attached screenshots).

I think the bug might lie in the definition of web-mode-buffer-narrowed-p, as on Emacs 28.2 that returns nil whenever point is on a folded element (and I'm guessing it should return t, in this case).

unfolded-element folded-element
fxbois commented 1 year ago

Thx for the report, would you have a fix for this issue ?

jmckalex commented 1 year ago

Dear Francois-Xavier,

Yes — there is a fix for the issue. Please see the forwarded message below!

Best wishes,

Jason

On 5 Sep 2023, at 16:29, fxbois @.***> wrote:

Thx for the report, would you have a fix for this issue ?

— Reply to this email directly, view it on GitHubhttps://github.com/fxbois/web-mode/issues/1274#issuecomment-1706841835, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADFPKYCHG6TL5RRGEVELM33XY5AOPANCNFSM6AAAAAAXGWSU4I. You are receiving this because you authored the thread.Message ID: @.***>

Begin forwarded message:

From: Jason Alexander @.> Subject: Re: Patch for bug(?) in web-mode.el Date: 3 May 2023 at 11:10:02 BST To: Bois Francois-Xavier @.>

Hi Francois-Xavier,

Thanks for getting back to me! I’ve included below a further modified version of web-mode-column-show which improves on the one I sent previously.

In my last email, I noted that if the element showing its column had folded child elements inside, there would be a spurious overlay placed at the end of the folded child elements. This version fixes that (see attached screenshot). The way it works is that it tracks the current line number when an overlay is being created. If the current line equals 1+ the previous line, the overlay is created. However, if the current line is more than 1+ the previous line, that means (I assume) that we have a folded child element which spans multiple lines. The overlay is not created in that case.

This new patch, plus calculating the number of overlays to insert differently (this is what the ‘line-delta’ variable tracks) results in the column overlays to be displayed correctly. This is probably not the most elegant solution, but it works.

As an aside: there’s another “bug” I found in web-mode, but I have no idea what’s causing it. Whenever I move the cursor over an element, and the tag is highlighted / its column is shown, I get the message “Mark set” appearing in the minibuffer. (You can see this in the attached screenshot.) This doesn’t cause any problems, exactly, but it means that any lisp code which puts a message in the minibuffer (e.g., as part of debugging) gets clobbered. I’ve tried changing all the save-excursion calls in web-mode to save-mark-and-excursion (because as of version 25.1 emacs no longer protects mark in save-excursion) but that doesn’t make any difference…

Many thanks!

Jason

(defun web-mode-column-show () (let ((index 0) overlay diff column line-to line-from line-delta regions (overlay-skip nil) last-line-no) (web-mode-column-hide) (setq web-mode-enable-current-column-highlight t) (save-mark-and-excursion (back-to-indentation) (setq column (current-column) line-to (web-mode-line-number)) (when (and (get-text-property (point) 'tag-beg) (member (get-text-property (point) 'tag-type) '(start end)) (web-mode-tag-match) (setq line-from (web-mode-line-number)) (not (= line-from line-to))) (when (> line-from line-to) (let (tmp) (setq tmp line-from) (setq line-from line-to) (setq line-to tmp)) ) ;when ;;(message "column(%S) line-from(%S) line-to(%S)" column line-from line-to) (goto-char (point-min)) (when (> line-from 1) (forward-line (1- line-from))) ;; Added by JMA (save-mark-and-excursion (let (start-point end-point) (goto-line line-from) (move-to-column column) (setq start-point (point)) (goto-line line-to) (move-to-column column) (setq end-point (point)) (setq line-delta (count-lines start-point end-point t)) (setq line-delta (+ line-delta (count-invisible-character-ranges start-point end-point)))) (setq line-to (+ line-from (1- line-delta)))) ;(message (format "Currently at line: %d" (line-number-at-pos))) (setq last-line-no (line-number-at-pos)) ;; end JMA add (while (<= line-from line-to) (setq overlay (web-mode-column-overlay-factory index)) (setq diff (- (line-end-position) (point))) (cond ((or (and (= column 0) (= diff 0)) (> column diff)) (end-of-line) (move-overlay overlay (point) (point)) (overlay-put overlay 'after-string (concat (if (> column diff) (make-string (- column diff) ?\s) "") (propertize " " 'font-lock-face 'web-mode-current-column-highlight-face) ) ;concat ) ) (t (move-to-column column) (overlay-put overlay 'after-string nil) (move-overlay overlay (point) (1+ (point))) ) ) ;cond (setq line-from (1+ line-from)) (forward-line) ;; JMA ADD ;(message (format "Currently at line: %d" (line-number-at-pos))) (if (not (= (1+ last-line-no) (line-number-at-pos))) (delete-overlay overlay)) (setq last-line-no (line-number-at-pos)) ;; END JMA ADD (setq index (1+ index)) ) ;while ) ;when ) ;save-mark-and-excursion ) ;let )

— Prof. J. McKenzie Alexander Department of Philosophy, Logic and Scientific Method London School of Economics and Political Science Houghton Street, London WC2A 2AE

[Screenshot 2023-05-03 at 10.57.16.png]

On 3 May 2023, at 08:40, Bois Francois-Xavier @.***> wrote:

Alexander, very sorry not to have answered your mail. Very busy with my company at the moment. I'll try to look at your mail this weekend. Thank you for your work on this. Best

Le ven. 21 avr. 2023 à 20:27, Alexander,J @.**@.>> a écrit : Hi Francois-Xavier,

Apologies for emailing you directly, but I wasn’t sure what the best way was to submit a possible bug patch regarding web-mode.el.

You might have seen that I logged an issue on GitHub regarding the fact that, on Emacs 28.2, highlighting the current column didn’t work well if the element was folded. I’ve managed to fix that as follows.

First I defined the following function to check if the element at point is folded (it returns nil if point isn’t on an element). This assumes that a folded element won’t have empty content — which I think is OK because, if it did, why would the user bother folding it?

(defun element-at-point-folded-p () (let* ((boundaries (web-mode-element-boundaries (point))) (end-of-start-tag (cdr (car boundaries)))) (when boundaries (get-text-property (1+ end-of-start-tag) 'invisible))))

Second, I then used that function in a conditional test in the following snippet from web-mode-on-post-command:

(when (and web-mode-enable-current-column-highlight
           (not (web-mode-buffer-narrowed-p)))
  (if (not (element-at-point-folded-p))
      (web-mode-column-show)))

That solved the problem I logged on GitHub. However, it then revealed another issue: highlighting the current column didn’t work well if the element whose column was being highlighted contained child elements which also were folded. In this case, the column highlight would extend below the end of the element because web-mode-column-show didn’t take into account the possibility of folded child elements.

I fixed that problem, too (see attached screenshot - the tags displayed in boxes are folded elements) but the code isn’t pretty! I altered the definition of web-mode-column-show as follows:

  1. I added a save-excursion block which calculates line-delta, the difference between the actual number of lines the element spans and the number of visible lines in the buffer. (This could probably be done more efficiently by changing the definition of web-mode-line-number, but I couldn’t figure out how that function worked.)

  2. line-delta was then used to compute the value of line-to.

  3. However, since web-mode-column-show adds a spurious overlay character to the end of each folded child in addition to the column which is to be highlighted (as you can see from the screenshot), I fixed this with a hack that calculated how many contiguous regions of characters with the text-property ‘invisible there were in the element — since that was a quick-and-dirty way of determining how many folded child elements there were. This was done using the function count-invisible-character-ranges, which is defined below.

Taken together, this seems to fix the original issue I logged as well as the issue of highlighting columns for elements containing folded child elements. There is still one minor further issue: if the element contains a folded child element that doesn’t span a newline, the column highlighting will still not be right… but I don’t know how best to handle this minor condition!

(defun web-mode-column-show () (let ((index 0) overlay diff column line-to line-from line-delta regions ) (web-mode-column-hide) (setq web-mode-enable-current-column-highlight t) (save-excursion (back-to-indentation) (setq column (current-column) line-to (web-mode-line-number)) (when (and (get-text-property (point) 'tag-beg) (member (get-text-property (point) 'tag-type) '(start end)) (web-mode-tag-match) (setq line-from (web-mode-line-number)) (not (= line-from line-to))) (when (> line-from line-to) (let (tmp) (setq tmp line-from) (setq line-from line-to) (setq line-to tmp)) ) ;when ;;(message "column(%S) line-from(%S) line-to(%S)" column line-from line-to) (goto-char (point-min)) (when (> line-from 1) (forward-line (1- line-from))) ;; Added by JMA (save-excursion (let (start-point end-point) (goto-line line-from) (move-to-column column) (setq start-point (point)) (goto-line line-to) (move-to-column column) (setq end-point (point)) (setq line-delta (count-lines start-point end-point t)) (setq line-delta (+ line-delta (count-invisible-character-ranges start-point end-point)))) (setq line-to (+ line-from (1- line-delta)))) ;; end JMA add (while (<= line-from line-to) (setq overlay (web-mode-column-overlay-factory index)) (setq diff (- (line-end-position) (point))) (cond ((or (and (= column 0) (= diff 0)) (> column diff)) (end-of-line) (move-overlay overlay (point) (point)) (overlay-put overlay 'after-string (concat (if (> column diff) (make-string (- column diff) ?\s) "") (propertize " " 'font-lock-face 'web-mode-current-column-highlight-face) ) ;concat ) ) (t (move-to-column column) (overlay-put overlay 'after-string nil) (move-overlay overlay (point) (1+ (point))) ) ) ;cond (setq line-from (1+ line-from)) (forward-line) (setq index (1+ index)) ) ;while ) ;when ) ;save-excursion ) ;let )

Here’s the function used to determine how many folded child elements there are:

(defun count-invisible-character-ranges (min max) (interactive "r") (let ((count 0) (current-pos min)) (save-excursion (while (<= current-pos max) (goto-char current-pos) (if (get-text-property current-pos 'invisible) (progn (setq count (1+ count)) (setq current-pos (1+ current-pos)) (while (and (<= current-pos max) (get-text-property current-pos 'invisible)) (setq current-pos (1+ current-pos)))) (setq current-pos (1+ current-pos))))) count))

Many thanks for writing web-mode! It’s very, very useful.

Best wishes,

Jason

— Prof. J. McKenzie Alexander Department of Philosophy, Logic and Scientific Method London School of Economics and Political Science Houghton Street, London WC2A 2AE

<Screenshot 2023-04-21 at 19.18.06.png>

-- François-Xavier Bois Kernix - Fondateur et Dirigeant 6 rue Lalande 75014 PARIS

fxbois commented 1 year ago

I've pushed your fix (I must confess I haven't tested)

jmckalex commented 1 year ago

It works on my computer!

Sent from my iPhone

On 5 Sep 2023, at 20:45, fxbois @.***> wrote:



I've pushed your fix (I must confess I haven't tested)

— Reply to this email directly, view it on GitHubhttps://github.com/fxbois/web-mode/issues/1274#issuecomment-1707215748, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADFPKYA7FBGFWUSCTWNV6UTXY56NTANCNFSM6AAAAAAXGWSU4I. You are receiving this because you authored the thread.Message ID: @.***>