Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Nicolas Goaziou]

Hello,

Yasushi SHOJI  writes:

> Sure.  Attached.

Applied. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Yasushi SHOJI]

Hi,

On Thu, 02 Jun 2016 02:34:31 +0900,
Nicolas Goaziou wrote:
> 
> > Would you remove the extra section marker (";;; Tags") thing when
> > merge to master?  It merges cleanly from maint to master but leaves
> > the marker.
> 
> I'll take care of the merging back to master.

Thank you.

> > please pull from:
> >
> >   https://github.com/yashi/org-mode tag-width-fix
> 
> I'd rather have the patch with the commit message as an attachment, if
> you don't mind.

Sure.  Attached.

Thanks,
-- 
 yashi

From b57dee1c44d8dd6979c5a6953ad12fdb487d5092 Mon Sep 17 00:00:00 2001
From: Yasushi SHOJI 
Date: Tue, 31 May 2016 16:25:42 +0900
Subject: [PATCH 1/2] org: Fix tag width calculation for multi-column chars
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some characters have multiple column width.  Calculating string width
with points gives a wrong value than actual display width.  Use
`string-width' instead.

Here is an ECM for this problem.  `M-x org-update-statistics-cookies` or
`C-c #` on bar moves the tag on the headline.

* foo [0/0] :abc:
** child
* bar [0/0]  :日本語:
** child
12345678901234567890123456789012345678901234567890123456789012345678901234567890
 1 2 3 4 5 6 7 8
---
 lisp/org.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index a9412ea..15f851d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -14923,7 +14923,7 @@ If ONOFF is `on' or `off', don't toggle but set to this 
state."
 (if(and (looking-at (org-re ".*?\\([ 
\t]+\\)\\(:[[:alnum:]_@#%:]+:\\)[ \t]*$"))
 (< pos (match-beginning 2)))
(progn
- (setq tags-l (- (match-end 2) (match-beginning 2)))
+ (setq tags-l (string-width (match-string 2)))
  (goto-char (match-beginning 1))
  (insert " ")
  (delete-region (point) (1+ (match-beginning 2)))
-- 
2.8.1

From 4cbd72931e7982878334c92abb08272f11ae0f32 Mon Sep 17 00:00:00 2001
From: Yasushi SHOJI 
Date: Tue, 31 May 2016 16:25:42 +0900
Subject: [PATCH 2/2] org: Add test for tag width calculation

This is a simple unit test case for the previous fix.
---
 testing/lisp/test-org.el | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index db7e525..4873fc2 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -4132,6 +4132,28 @@ Paragraph"
(org-occur "A" nil (lambda () (equal (org-get-heading) "H2")))
 
 
+;;; Tags
+
+(ert-deftest test-org/tag-align ()
+  "Test `org-align-tags-here' with different display width."
+  (should
+   ;;  12345678901234567890
+   (equal "* Test :abc:"
+ (org-test-with-temp-text "* Test :abc:"
+   (let ((org-tags-column -20)
+ (indent-tabs-mode nil))
+(org-fix-tags-on-the-fly))
+   (buffer-string
+  (should
+   ;;  12345678901234567890
+   (equal "* Test  :日本語:"
+ (org-test-with-temp-text "* Test :日本語:"
+   (let ((org-tags-column -20)
+ (indent-tabs-mode nil))
+(org-fix-tags-on-the-fly))
+   (buffer-string)
+
+
 ;;; Timestamps API
 
 (ert-deftest test-org/time-stamp ()
-- 
2.8.1

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Nicolas Goaziou]

Hello,

Yasushi SHOJI  writes:

> Ah, the patch was intended for the branch maint.

You're right, Tags section doesn't exist in in maint, so all is good.

> Would you remove the extra section marker (";;; Tags") thing when
> merge to master?  It merges cleanly from maint to master but leaves
> the marker.

I'll take care of the merging back to master.

> please pull from:
>
>   https://github.com/yashi/org-mode tag-width-fix

I'd rather have the patch with the commit message as an attachment, if
you don't mind.

Thank you !


Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Rasmus]

Samuel Wales  writes:

> i wonder if this also fixes tags for variable pitch (proportional)
> fonts.  i have been having great success with variable pitch fonts and
> it would be great if this fixes tags for them too.

No.


https://www.gnu.org/software/emacs/manual/html_node/elisp/Size-of-Displayed-Text.html


You'd need pixel alignment.

Here’s a version that aligns the left part pf tags.

(defun org-align-tags-here ( arg)
  ;; Assumes that this is a headline
  "Align tags on the current headline to TO-COL."
  (save-excursion
(when (org-at-heading-p)
  (beginning-of-line)
  (re-search-forward org-complex-heading-regexp (line-end-position) t)
  (when (match-end 5) ; There’s a tag.
(goto-char (match-end 4))
(delete-region (point) (match-beginning 5))
(insert (propertize
 (make-string (- (abs org-tags-column) (current-column)
 (- (match-end 5) (match-beginning 5)))
  ?\s)
 ;; one would need somehow intelligently figure out
 ;; the maximum pixel number.
 'display '(space :align-to (600


Questions if we were to pixel align:

- Can we drop right alignment of tags? (i.e. org-tags-column < 0).
- We might need to know the pixel width of :tags: anyway to set the width
  of the white space to "right-margin" - "width-of-tags".  Is this robust
  to resizing?  Or is there a "\hfill"-like property for Emacs space?
- How to determine max width on "auto-filled" file?  Go through each line
  or just make it a custom value?

Perhaps a nice "conservative" way is to introduce a org-tags-column-pixel,
but if we can find a good "automatic" way that’s nicer IMO.

Rasmus

-- 
The second rule of Fight Club is: You do not talk about Fight Club

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Yasushi SHOJI]

Hi Samuel,

On Wed, 01 Jun 2016 06:06:02 +0900,
Samuel Wales wrote:
> 
> i wonder if this also fixes tags for variable pitch (proportional)
> fonts.  i have been having great success with variable pitch fonts and
> it would be great if this fixes tags for them too.

This fix is nothing to do with propotional fonts.  What kind of
problem do you have right now?  Any screen shot or an ECM?

If you are refering to the Ambiguous width defined in UAX#11, it might.
-- 
   yashi

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Yasushi SHOJI]

Hi Nicolas,

On Wed, 01 Jun 2016 05:02:58 +0900,
Nicolas Goaziou wrote:
> Yasushi SHOJI  writes:
> > Let me know if I miss something.
> 
> Could you send it again with an appropriate commit message and using git
> format-patch?

Oops.  Sorry about the format=flowed.  Not sure how I got that, but
might be gmail thing.  I'll use NNTP or git pull request next time.

> Also, there is already a "Tags" section in "test-org.el". Could you add
> your tests there instead of creating a new section?

Ah, the patch was intended for the branch maint.

Would you remove the extra section marker (";;; Tags") thing when
merge to master?  It merges cleanly from maint to master but leaves
the marker.

please pull from:

  https://github.com/yashi/org-mode tag-width-fix

ps. If you want me to change it for master, let me know.  I'll do it.
--
yashi

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Samuel Wales]

i wonder if this also fixes tags for variable pitch (proportional)
fonts.  i have been having great success with variable pitch fonts and
it would be great if this fixes tags for them too.

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  And
ANYBODY can get it.

Denmark: free Karina Hansen NOW.

Re: [O] [PATCH] org: Fix tag width calculation for multi-column chars

[Nicolas Goaziou]

Hello,

Yasushi SHOJI  writes:

> Some characters have multiple column width.  Calculating string width
> with points gives a wrong value than actual display width.  Use
> `string-width' instead.
>
> Here is an ECM for this problem.  `M-x org-update-statistics-cookies` or
> `C-c #` on bar moves the tag on the headline.
>
> * foo [0/0]   :abc:
> ** child
> * bar [0/0]:日本語:
> ** child
> 12345678901234567890123456789012345678901234567890123456789012345678901234567890
>  1 2 3 4 5 6 7
>  8
>
> Simple test case is also included.

Thank you.

> Let me know if I miss something.

Could you send it again with an appropriate commit message and using git
format-patch?

Also, there is already a "Tags" section in "test-org.el". Could you add
your tests there instead of creating a new section?

Regards,

-- 
Nicolas Goaziou