Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Jakub Szypulka]

Reproduction of bug: Open an org-file in org-mode with auto-indent turned on while using a non-monospaced font.
Expected result: The headings align with the text contents.
Actual result: The headings do not align with the text contents.

This has been originally filed two years ago:
http://article.gmane.org/gmane.emacs.orgmode/64775/match=indent+misalign

There is a proposed fix: replace the blankspace used to indent a line, defined in org-indent.el, with a star ('*').
Source: http://emacs.stackexchange.com/questions/7429/how-to-customize-org-mode-indentation
This link also includes screenshots that illustrate the problem.

Thanks in advance for fixing!

Emacs  : GNU Emacs 24.5.1 (x86_64-apple-darwin13.4.0, NS apple-appkit-1265.21) of 2015-04-10 on builder10-9.porkrind.org
Package: Org-mode version 8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Nicolas Goaziou]

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

Hello,

Jakub Szypulka <jakub@szypulka.de> writes:

> Reproduction of bug: Open an org-file in org-mode with auto-indent turned on while using a non-monospaced font.
> Expected result: The headings align with the text contents.
> Actual result: The headings do not align with the text contents.
>
> This has been originally filed two years ago:
> http://article.gmane.org/gmane.emacs.orgmode/64775/match=indent+misalign
>
> There is a proposed fix: replace the blankspace used to indent a line, defined in org-indent.el, with a star ('*').
> Source: http://emacs.stackexchange.com/questions/7429/how-to-customize-org-mode-indentation
> This link also includes screenshots that illustrate the problem.

Thank you for reporting it.

Unfortunately, the proposed fix isn't sufficient as alignment is still
broken in plain lists, with `visual-line-mode'.

Does the attached patch work for you?


Regards,

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-indent-Fix-indentation-with-proportional-font.patch --]
[-- Type: text/x-diff, Size: 9734 bytes --]

From 95bfd6529664994554b60cbc22d2e26bb3158a23 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Thu, 20 Aug 2015 13:44:36 +0200
Subject: [PATCH] org-indent: Fix indentation with proportional font

* lisp/org-indent.el (org-indent-max):
(org-indent-max-levels):
(org-indent-strings):
(org-indent-stars): Remove unused variables.

(org-indent-initialize): Remove function.

(org-indent-boundary-char): Remove unnecessary comment.  Do not rely on
function above.

(org-indent): Make sure characters used for virtual indentation are
invisible since they are not necessarily white spaces.

(org-indent-set-line-properties): Fix indentation with proportional
font, i.e., do not use only white spaces to indent.
(org-indent-add-properties): Apply changes above.

Reported-by: Jakub Szypulka <jakub@szypulka.de>
<http://permalink.gmane.org/gmane.emacs.orgmode/100252>
---
 lisp/org-indent.el | 128 ++++++++++++++++++++---------------------------------
 1 file changed, 47 insertions(+), 81 deletions(-)

diff --git a/lisp/org-indent.el b/lisp/org-indent.el
index c8d3325..868ef5f 100644
--- a/lisp/org-indent.el
+++ b/lisp/org-indent.el
@@ -52,20 +52,6 @@
   :tag "Org Indent"
   :group 'org)
 
-(defconst org-indent-max 40
-  "Maximum indentation in characters.")
-(defconst org-indent-max-levels 20
-  "Maximum added level through virtual indentation, in characters.
-
-It is computed by multiplying `org-indent-indentation-per-level'
-minus one by actual level of the headline minus one.")
-
-(defvar org-indent-strings nil
-  "Vector with all indentation strings.
-It will be set in `org-indent-initialize'.")
-(defvar org-indent-stars nil
-  "Vector with all indentation star strings.
-It will be set in `org-indent-initialize'.")
 (defvar org-indent-inlinetask-first-star (org-add-props "*" '(face org-warning))
   "First star of inline tasks, with correct face.")
 (defvar org-indent-agent-timer nil
@@ -92,15 +78,12 @@ This is used locally in each buffer being initialized.")
 It is modified by `org-indent-notify-modified-headline'.")
 
 
-(defcustom org-indent-boundary-char ?\   ; comment to protect space char
+(defcustom org-indent-boundary-char ?\s
   "The end of the virtual indentation strings, a single-character string.
 The default is just a space, but if you wish, you can use \"|\" or so.
 This can be useful on a terminal window - under a windowing system,
-it may be prettier to customize the org-indent face."
+it may be prettier to customize the `org-indent' face."
   :group 'org-indent
-  :set (lambda (var val)
-	 (set var val)
-	 (and org-indent-strings (org-indent-initialize)))
   :type 'character)
 
 (defcustom org-indent-mode-turns-off-org-adapt-indentation t
@@ -121,30 +104,12 @@ turn on `org-hide-leading-stars'."
   :group 'org-indent
   :type 'integer)
 
-(defface org-indent
-  (org-compatible-face nil nil)
+(defface org-indent '((t (:inherit org-hide)))
   "Face for outline indentation.
 The default is to make it look like whitespace.  But you may find it
 useful to make it ever so slightly different."
   :group 'org-faces)
 
-(defun org-indent-initialize ()
-  "Initialize the indentation strings."
-  (setq org-indent-strings (make-vector (1+ org-indent-max) nil))
-  (setq org-indent-stars (make-vector (1+ org-indent-max) nil))
-  (aset org-indent-strings 0 nil)
-  (aset org-indent-stars 0 nil)
-  (loop for i from 1 to org-indent-max do
-	(aset org-indent-strings i
-	      (org-add-props
-		  (concat (make-string (1- i) ?\ )
-			  (char-to-string org-indent-boundary-char))
-		  nil 'face 'org-indent)))
-  (loop for i from 1 to org-indent-max-levels do
-	(aset org-indent-stars i
-	      (org-add-props (make-string i ?*)
-		  nil 'face 'org-hide))))
-
 (defsubst org-indent-remove-properties (beg end)
   "Remove indentations between BEG and END."
   (org-with-silent-modifications
@@ -174,7 +139,6 @@ during idle time."
    (org-indent-mode
     ;; mode was turned on.
     (org-set-local 'indent-tabs-mode nil)
-    (or org-indent-strings (org-indent-initialize))
     (org-set-local 'org-indent-initial-marker (copy-marker 1))
     (when org-indent-mode-turns-off-org-adapt-indentation
       (org-set-local 'org-adapt-indentation nil))
@@ -281,34 +245,40 @@ a time value."
 	   (setq org-indent-agentized-buffers
 		 (delq buffer org-indent-agentized-buffers))))))))
 
-(defsubst org-indent-set-line-properties (l w h)
+(defun org-indent-set-line-properties (level indentation &optional heading)
   "Set prefix properties on current line an move to next one.
 
-Prefix properties `line-prefix' and `wrap-prefix' in current line
-are set to, respectively, length L and W.
-
-If H is non-nil, `line-prefix' will be starred.  If H is
-`inline', the first star will have `org-warning' face.
-
-Assume point is at beginning of line."
-  (let ((line (cond
-	       ((eq 'inline h)
-		(let ((stars (aref org-indent-stars
-				   (min l org-indent-max-levels))))
-		  (and stars
-		       (if (org-bound-and-true-p org-inlinetask-show-first-star)
-			   (concat org-indent-inlinetask-first-star
-				   (substring stars 1))
-			 stars))))
-	       (h (aref org-indent-stars
-			(min l org-indent-max-levels)))
-	       (t (aref org-indent-strings
-			(min l org-indent-max)))))
-	(wrap (aref org-indent-strings (min w org-indent-max))))
+LEVEL is the current level of heading.  INDENTATION is the
+expected indentation when wrapping line.
+
+When optional argument HEADING is non-nil, assume line is at
+a heading.  Moreover, if is is `inlinetask', the first star will
+have `org-warning' face."
+  (let* ((stars (if (<= level 1) ""
+		  (make-string (* org-indent-indentation-per-level
+				  (1- level))
+			       ?*)))
+	 (line
+	  (cond
+	   ((and (org-bound-and-true-p org-inlinetask-show-first-star)
+		 (eq heading 'inlinetask))
+	    (concat org-indent-inlinetask-first-star
+		    (org-add-props (substring stars 1) nil 'face 'org-hide)))
+	   (heading (org-add-props stars nil 'face 'org-hide))
+	   (t (concat (org-add-props (concat stars (make-string level ?*))
+			  nil 'face 'org-indent)
+		      (char-to-string org-indent-boundary-char)))))
+	 (wrap
+	  (org-add-props
+	      (concat stars
+		      (make-string level ?*)
+		      (if heading " "
+			(make-string (+ indentation (min level 1)) ?\s)))
+	      nil 'face 'org-indent)))
     ;; Add properties down to the next line to indent empty lines.
-    (add-text-properties (point) (min (1+ (point-at-eol)) (point-max))
+    (add-text-properties (line-beginning-position) (line-beginning-position 2)
 			 `(line-prefix ,line wrap-prefix ,wrap)))
-  (forward-line 1))
+  (forward-line))
 
 (defun org-indent-add-properties (beg end &optional delay)
   "Add indentation properties between BEG and END.
@@ -328,12 +298,10 @@ stopped."
      ;;    inline task or not.
      (let* ((case-fold-search t)
 	    (limited-re (org-get-limited-outline-regexp))
-	    (added-ind-per-lvl (abs (1- org-indent-indentation-per-level)))
 	    (pf (save-excursion
 		  (and (ignore-errors (let ((outline-regexp limited-re))
 					(org-back-to-heading t)))
-		       (+ (* org-indent-indentation-per-level
-			     (- (match-end 0) (match-beginning 0) 2)) 2))))
+		       (- (match-end 0) (match-beginning 0) 1))))
 	    (pf-inline (and (featurep 'org-inlinetask)
 			    (org-inlinetask-in-task-p)
 			    (+ (* org-indent-indentation-per-level
@@ -354,38 +322,36 @@ stopped."
 	   ((and delay (time-less-p time-limit (current-time)))
 	    (setq org-indent-agent-resume-timer
 		  (run-with-idle-timer
-		   (time-add (current-idle-time)
-			     org-indent-agent-resume-delay)
+		   (time-add (current-idle-time) org-indent-agent-resume-delay)
 		   nil #'org-indent-initialize-agent))
 	    (throw 'interrupt (point)))
 	   ;; Headline or inline task.
 	   ((looking-at org-outline-regexp)
-	    (let* ((nstars (- (match-end 0) (match-beginning 0) 1))
-		   (line (* added-ind-per-lvl (1- nstars)))
-		   (wrap (+ line (1+ nstars))))
+	    (let* ((nstars (- (match-end 0) (match-beginning 0) 1)))
 	      (cond
 	       ;; Headline: new value for PF.
 	       ((looking-at limited-re)
-		(org-indent-set-line-properties line wrap t)
-		(setq pf wrap))
+		(org-indent-set-line-properties nstars 0 t)
+		(setq pf nstars))
 	       ;; End of inline task: PF-INLINE is now nil.
 	       ((looking-at "\\*+ end[ \t]*$")
-		(org-indent-set-line-properties line wrap 'inline)
+		(org-indent-set-line-properties nstars 0 'inlinetask)
 		(setq pf-inline nil))
 	       ;; Start of inline task.  Determine if it contains
 	       ;; text, or if it is only one line long.  Set
 	       ;; PF-INLINE accordingly.
-	       (t (org-indent-set-line-properties line wrap 'inline)
-		  (setq pf-inline (and (org-inlinetask-in-task-p) wrap))))))
+	       (t (org-indent-set-line-properties nstars 0 'inlinetask)
+		  (setq pf-inline (and (org-inlinetask-in-task-p) nstars))))))
 	   ;; List item: `wrap-prefix' is set where body starts.
 	   ((org-at-item-p)
 	    (let* ((line (or pf-inline pf 0))
-		   (wrap (+ (org-list-item-body-column (point)) line)))
-	      (org-indent-set-line-properties line wrap nil)))
+		   (wrap (org-list-item-body-column (point))))
+	      (org-indent-set-line-properties
+	       (or pf-inline pf 0)
+	       (org-list-item-body-column (point)))))
 	   ;; Normal line: use PF-INLINE, PF or nil as prefixes.
-	   (t (let* ((line (or pf-inline pf 0))
-		     (wrap (+ line (org-get-indentation))))
-		(org-indent-set-line-properties line wrap nil))))))))))
+	   (t (org-indent-set-line-properties
+	       (or pf-inline pf 0) (org-get-indentation))))))))))
 
 (defun org-indent-notify-modified-headline (beg end)
   "Set `org-indent-modified-headline-flag' depending on context.
-- 
2.5.0

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Jakub Szypulka]

Hello Nicolas,

---
Nicolas Goaziou <mail@nicolasgoaziou.fr>, 2015-08-20 13:56
> Hello,
>
> Jakub Szypulka <jakub@szypulka.de> writes:
>
>> Reproduction of bug: Open an org-file in org-mode with auto-indent turned on while using a non-monospaced font.
>> Expected result: The headings align with the text contents.
>> Actual result: The headings do not align with the text contents.
>>
>> This has been originally filed two years ago:
>> http://article.gmane.org/gmane.emacs.orgmode/64775/match=indent+misalign
>>
>> There is a proposed fix: replace the blankspace used to indent a line, defined in org-indent.el, with a star ('*').
>> Source: http://emacs.stackexchange.com/questions/7429/how-to-customize-org-mode-indentation
>> This link also includes screenshots that illustrate the problem.
>
> Thank you for reporting it.
>
> Unfortunately, the proposed fix isn't sufficient as alignment is still
> broken in plain lists, with `visual-line-mode'.
>
> Does the attached patch work for you?

Yes it does, I'm impressed! Thanks a lot for the quick fix!

I understand those changed will be included by default in the next org versions?

Cheers,
Jakub

>
>
> Regards,

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Nicolas Goaziou]

Jakub Szypulka <jakub@szypulka.de> writes:

> Yes it does, I'm impressed! Thanks a lot for the quick fix!

Applied then. Thank you for the feedback.

> I understand those changed will be included by default in the next org
> versions?

Yes, they will.

Regards,

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Jakub Szypulka]

Wait, it seems your patch also fixed another issue!

Until now, org-mode would only indent text correctly up until a certain level (not sure exactly, but approx 8-10 stars), and would then bug out: the heading, when demoted further, would be correctly indented by a certain amount of characters (`org-indent-indentation-per-level' probably), but the text would be indented by a *lesser* amount of characters, resulting in a slight misalign at every demote step, that would grow bigger and bigger the more you demoted.

Now I can demote even beyond the right screen edge, and the text keeps following.

Maybe you can add this information to the commit or the changelog in case anyone else was affected by this.

Thanks again!
Jakub

> --- Original message ---
> Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote on Thursday, 20 August 2015 at 21:56:
>
> Jakub Szypulka <jakub@szypulka.de> writes:
>
>> Yes it does, I'm impressed! Thanks a lot for the quick fix!
>
> Applied then. Thank you for the feedback.
>
>> I understand those changed will be included by default in the next org
>> versions?
>
> Yes, they will.
>
> Regards,

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Eric S Fraga]

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

Hi,

I note that this bug was addressed in a recent commit.  However, now
indent mode *with* a monospaced font has some extra space (the width of
a *) at the start of every headline except top level headings.  Also
seems to affect text that comes before the first headline.  Image
attached shows what it looks like.

Thanks,
eric

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 25.0.50.2, Org release_8.3.1-166-g5bfdfc

[-- Attachment #2: screendump-20150825133819.png --]
[-- Type: image/png, Size: 38257 bytes --]

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Eric S Fraga]

On Tuesday, 25 Aug 2015 at 13:39, Eric S Fraga wrote:
> Hi,
>
> I note that this bug was addressed in a recent commit.  However, now
> indent mode *with* a monospaced font has some extra space (the width of
> a *) at the start of every headline except top level headings.  Also
> seems to affect text that comes before the first headline.  Image
> attached shows what it looks like.
>
> Thanks,
> eric


Sorry for responding to my own post but I need to clarify two things:

1. this problem appears with emacs -Q and latest org from git
2. the problem was introduced *before* the recent fix to indentation

thanks,
eric

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 25.0.50.2, Org release_8.3.1-166-g5bfdfc

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Nicolas Goaziou]

Hello,

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> I note that this bug was addressed in a recent commit.  However, now
> indent mode *with* a monospaced font has some extra space (the width of
> a *) at the start of every headline except top level headings.  Also
> seems to affect text that comes before the first headline.

This is fixed. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: Bug: Org-indent does not align headings with text when using non-monospaced fonts [8.3.1 (8.3.1-16-gf6aa53-elpa @ /Users/cube/.emacs.d/elpa/org-20150810/)]

[Eric S Fraga]

On Tuesday, 25 Aug 2015 at 15:05, Nicolas Goaziou wrote:
> Hello,
>
> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
>> I note that this bug was addressed in a recent commit.  However, now
>> indent mode *with* a monospaced font has some extra space (the width of
>> a *) at the start of every headline except top level headings.  Also
>> seems to affect text that comes before the first headline.
>
> This is fixed. Thank you.

Indeed!  Thank you for such a quick response.  :-)

-- 
: Eric S Fraga (0xFFFCF67D), Emacs 25.0.50.2, Org release_8.3.1-166-g5bfdfc