Re: variable-pitch-mode misaligns org-mode heading tags

[Adam Spiers <orgmode@adamspiers.org>]

[-- Attachment #1: Type: text/plain, Size: 3326 bytes --]

On Wed, Sep 16, 2020 at 03:03:02PM -0400, Jeff Filipovits wrote: 
>It looks like (window-text-pixel-size) could be used to calculate the 
>pixel length of the tags list? 

Ahah!  This indeed did the trick!  I have no idea how I missed this 
handy function previously when I was scouring the manual... 

>I am having trouble deciphering the 
>manual (https://www.gnu.org/software/emacs/manual/html_node/elisp/Pixel-Specification.html#Pixel-Specification) 
>for pixel specification for spaces, though. The right alignment 
>specification for some reason sends the tags to the next line, as do 
>most other solutions that I would expect to align the text to the 
>right side of the window. 
>
>I can experiment more in a couple days, but in the meantime maybe 
>someone smarter than me give some hints on how to use the pixel 
>specification properties. 

I've actually managed it get it working now!  See the attached patch. 
However unfortunately it is not ready to be merged yet.  Firstly, it 
breaks `test-org/tag-align'.  Secondly, since this changes the method 
of alignment from simply using raw spaces to using a single space with 
a display text property, when the file is saved and reloaded, it just 
displays a single space and the alignment is lost.  Another unfortunate 
side-effect is that if the file is simultaneously displayed in a graphical 
window and a text window via the same emacs server (e.g. via emacsclient -t), 
then one of the two windows will have incorrect alignment. 

So I think the correct fix will be to keep the original number of 
spaces, but use a display text property to redisplay those spaces as a 
single space with the given width.  I'm guessing that the display text 
property will be ignored on text-only (tty) windows, fixing both 
problems in one go. 

So I'll take another stab at this soon, if Bastien or some other Org 
guru can confirm I'm on the right track. 

>>BTW I tried your code and for some reason it didn't insert any space 
>>for me, but I didn't look into that yet. 
>
>The way it’s written it would only reduce the gap between the headline 
>and tags to a space, and it assumes there are multiple spaces there 
>already. If there’s no space between the two, I don’t think it’ll 
>insert one. Probably not the best way as it was thrown together to 
>test the text property fix. 

I figured out that it wasn't working because my `org-tags-column' is a 
negative value in order to align flush-right, and your code didn't 
support that. 

>I accepted long ago that the solution to using a variable pitch font 
>for org headings was that the tags would not be aligned to the right 
>and never looked back, so maybe this is not worth the price of fixing 
>it if it is messy. And diving down to calculating the pixel width of 
>text seems like it’s getting pretty messy. 

Actually I think it ended up fairly clean.  Huge thanks to you for 
giving me the hint I needed!  I just adapted your code a bit.  I've 
marked you as a co-author in the commit message.  For now your 
contribution probably still falls into the category of "Tiny changes": 

     https://orgmode.org/worg/org-contribute.html#org9fbb342

However you might want to preemptively sign the copyright assignment 
papers: 

     https://orgmode.org/worg/org-contribute.html#copyrighted-contributors

[-- Attachment #2: 0001-org.el-Align-tags-using-specified-space-display-prop.patch --]
[-- Type: text/x-patch, Size: 4927 bytes --]

From 7655c32847d7abd9da7603b1a1a314b7d1b87ba5 Mon Sep 17 00:00:00 2001
From: Adam Spiers <orgmode@adamspiers.org>
Date: Wed, 16 Sep 2020 23:12:04 +0100
Subject: [PATCH] [WIP] org.el: Align tags using specified space display property
To: emacs-orgmode@gnu.org

Previously tags on heading lines were aligned using spaces, which
assumed a fixed width font.  However variable pitch fonts are becoming
increasingly popular, so ensure there is always a single space in
between the heading text and the (colon-delimited) list of tags, and
then if necessary use a display text property to specify the exact
width required by that space to align it in accordance with the value
of `org-tags-column' which the user has chosen:

  https://www.gnu.org/software/emacs/manual/html_node/elisp/Pixel-Specification.html#Pixel-Specification

If the value is positive, align flush-left; if negative, align
flush-right; and if zero, just leave a normal width space.

See the following links for the discussion threads leading to this
patch:

- https://lists.gnu.org/archive/html/emacs-orgmode/2020-09/msg00415.html
- https://gitlab.com/protesilaos/modus-themes/-/issues/85

Signed-off-by: Adam Spiers <orgmode@adamspiers.org>
Co-authored-by: Jeff Filipovits <jrfilipovits@gmail.com>
---
 lisp/org.el | 68 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 053635c85..e800eb642 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11831,35 +11831,45 @@ (defun org-toggle-tag (tag &optional onoff)
       res)))
 
 (defun org--align-tags-here (to-col)
-  "Align tags on the current headline to TO-COL.
-Assume point is on a headline.  Preserve point when aligning
-tags."
-  (when (org-match-line org-tag-line-re)
-    (let* ((tags-start (match-beginning 1))
-	   (blank-start (save-excursion
-			  (goto-char tags-start)
-			  (skip-chars-backward " \t")
-			  (point)))
-	   (new (max (if (>= to-col 0) to-col
-		       (- (abs to-col) (string-width (match-string 1))))
-		     ;; Introduce at least one space after the heading
-		     ;; or the stars.
-		     (save-excursion
-		       (goto-char blank-start)
-		       (1+ (current-column)))))
-	   (current
-	    (save-excursion (goto-char tags-start) (current-column)))
-	   (origin (point-marker))
-	   (column (current-column))
-	   (in-blank? (and (> origin blank-start) (<= origin tags-start))))
-      (when (/= new current)
-	(delete-region blank-start tags-start)
-	(goto-char blank-start)
-	(let ((indent-tabs-mode nil)) (indent-to new))
-	;; Try to move back to original position.  If point was in the
-	;; blanks before the tags, ORIGIN marker is of no use because
-	;; it now points to BLANK-START.  Use COLUMN instead.
-	(if in-blank? (org-move-to-column column) (goto-char origin))))))
+  "Align tags on the current headline to TO-COL.  Since TO-COL is
+derived from `org-tags-column', a negative value is interpreted as
+alignment flush-right, a positive value as flush-left, and 0 means
+insert a single space in between the headline and the tags.
+
+Assume point is on a headline.  Preserve point when aligning tags."
+  (save-excursion
+    (when (org-match-line org-tag-line-re)
+      (let* ((tags-start (match-beginning 1))
+             (tags-end (match-end 1))
+             (tags-pixel-width
+              (car (window-text-pixel-size (selected-window)
+                                           tags-start tags-end)))
+             (blank-start (progn
+                            (goto-char tags-start)
+                            (skip-chars-backward " \t")
+                            (point))))
+        ;; If there is more than one space between the headline and
+        ;; tags, delete the extra spaces.  Might be better to make the
+        ;; delete region one space smaller rather than inserting a new
+        ;; space?
+        (when (> tags-start (1+ blank-start))
+          (delete-region blank-start tags-start)
+          (goto-char blank-start)
+          (insert " "))
+        (if (= to-col 0)
+            ;; Just leave one normal space width
+            (remove-text-properties blank-start (1+ blank-start) '(display nil))
+          (let ((align-expr
+                 (if (> to-col 0)
+                     ;; Left-align positive values
+                     to-col
+                   ;; Right-align negative values by subtracting the
+                   ;; width of the tags.  Conveniently, the pixel
+                   ;; specification allows us to mix units,
+                   ;; subtracting a pixel width from a column number.
+                   `(- ,(- to-col) (,tags-pixel-width)))))
+            (put-text-property blank-start (1+ blank-start)
+                               'display `(space . (:align-to ,align-expr)))))))))
 
 (defun org-set-tags-command (&optional arg)
   "Set the tags for the current visible entry.
-- 
2.28.0