Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

buffer list layout irregular when buffer-name contain multi-byte character #170

Closed
luozengbin opened this Issue Dec 10, 2012 · 13 comments

Comments

Projects
None yet
2 participants
Contributor

luozengbin commented Dec 10, 2012

my screen capture

helm-buffer-list-with-multi-byte-string-01

for support multi-byte character in buffer list. The function helm-c-highlight-buffers that defined in helm-buffers.el should use (string-width STRING) instead (length SEQUENCE) to calculate truncbuf variable.

here is my solution

The following code to get substring with string width

(defun substring-by-width (str max-length)
  (loop for y = str then (substring y  0 (- (length y) 1)) do
        (if (<= (string-width y) max-length)
            (return
             (concat y (make-string
                        (- max-length
                           (string-width y)) ? )))
          nil)))

fix helm-c-highlight-buffers for support multi-byte character

(defun helm-c-highlight-buffers (buffers sources)
  "Transformer function to highlight BUFFERS list.
Should be called after others transformers i.e (boring buffers)."
  (loop ;; length of last buffer size string.
   ;; Start at ten, such a length should never be reach.
   ;; e.g 9999K, so the max should be 5 + a space = 6.
   .......
   for truncbuf = (if (> (string-width i) helm-buffer-max-length)
                      (concat (my-substring-by-width i helm-buffer-max-length)  ;; support  multi-byte character
                              "...")
                    (concat i (make-string
                               (- (+ helm-buffer-max-length 3)
                                  (string-width i)) ? )))
   .......
   ))

after fix

helm-buffer-list-with-multi-byte-string-02

Owner

thierryvolpiatto commented Dec 10, 2012

Thanks, looks good, can you submit a pull request?

Owner

thierryvolpiatto commented Dec 10, 2012

Please rename substring-by-width with helm prefix and move it to helm-utils.el.

luozengbin added a commit to luozengbin/helm that referenced this issue Dec 10, 2012

* helm-buffers.el: Issue #170 Support multi-byte character in buffer …
…list.

(helm-c-highlight-buffers): Use `helm-substring-by-width' calculate buffer name

thierryvolpiatto added a commit that referenced this issue Dec 11, 2012

Merge pull request #171 from luozengbin/Issue170
* helm-buffers.el: Issue #170 Support multi-byte character in buffer lis...

thierryvolpiatto added a commit that referenced this issue Dec 12, 2012

* helm-buffers.el (helm-c-highlight-buffers): Issue #170 Use truncate…
…-string-to-width instead of helm-substring-by-width.

* helm-utils.el: Remove helm-substring-by-width.
Owner

thierryvolpiatto commented Dec 12, 2012

@luozengbin I replaced helm-substring-by-width by truncate-string-to-width, it should do the same.
Can you check if your truncated buffers are displayed correctly?
Thanks.

Contributor

luozengbin commented Dec 12, 2012

@thierryvolpiatto

helm-substring-by-width and truncate-string-to-width not exactly the same.
checkout the fllowing test code

(string-width
 (helm-substring-by-width "1ああ" 4)    ; => "1あ "
 )                                      ; => 4

(string-width
 (truncate-string-to-width "1ああ" 4)   ; => "1あ"
 )                                      ; => 3

may be you can fix that like this.

(concat (truncate-string-to-width str width)
          (make-string
           (- width (string-width str)) ? ))
Owner

thierryvolpiatto commented Dec 13, 2012

luozengbin notifications@github.com writes:

helm-substring-by-width and truncate-string-to-width not exactly the same.
checkout the fllowing test code

(string-width
 (helm-substring-by-width "1ああ" 4)    ; => "1あ "
 )                                      ; => 4

(string-width
 (truncate-string-to-width "1ああ" 4)   ; => "1あ"
 )                                      ; => 3

may be you can fix that like this.

(concat (truncate-string-to-width str width)
          (make-string
           (- width (string-width str)) ? ))

Ok, thanks for the test, I will just reuse your
helm-substring-by-width, so revert the change.

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto added a commit that referenced this issue Dec 13, 2012

* helm-buffers.el: Issue #170 revert precedent changes.
* helm-utils.el: put back helm-substring-by-width.
Owner

thierryvolpiatto commented Dec 13, 2012

Well with truncate-string-to-width we can have the same result than with helm-substring-by-width with the 'padding' arg:

(truncate-string-to-width "1ああ" 4 nil ? )

but I will keep your code for now.

I still have no aligment though trying with a buffer named:
"サウンドステージ第話「ドキ!水着でプールで大ピンチなの」"
but I don't know which language it is.
can you send me here one of your buffer names?

Contributor

luozengbin commented Dec 13, 2012

truncate-string-to-width is seems good.
If the font settings are correct for specified language, then, should be aligned.
If you do not mind i can help you test that in japanese and chinese.

japanese buffer names
これはテストファイルです
01.全角と半角混在テストファイル

chinese buffer names
这是一个测试文件它的文件名很长
01.半角全角混合测试文件它的文件名很长

Owner

thierryvolpiatto commented Dec 13, 2012

Thanks for help, I tried with your buffer names and I still have not a good alignment.

Screenshot-helm-buffers

Owner

thierryvolpiatto commented Dec 13, 2012

The result is the same with:

(truncate-string-to-width i helm-buffer-max-length nil ? )

and

(helm-string-by-width i helm-buffer-max-length)

thierryvolpiatto added a commit that referenced this issue Dec 13, 2012

* helm-buffers.el (helm-c-highlight-buffers): Issue #170 fix it.
* helm-utils.el:
(helm-substring, helm-string-multibyte-p): New
(helm-substring-by-width): Use them.
Owner

thierryvolpiatto commented Dec 13, 2012

It is ok here with the buffer names you gave me, can you check on your side?
Thanks.

Contributor

luozengbin commented Dec 14, 2012

hi thierryvolpiatto! Please forgive my terrible english.

With this commit have not a good alignment on my side.
With the old commit its work fine. I guess the problem is font setting.
I'm using monosapce font on emacs. In CJK area most emacs user like use monospace font.

as my case under windows os

Takao Gothic font for japanese
Microsoft Yahei font for chinese

under linux os

VL Gothic font for japanese
wqy-microhei font for chinese

http://en.wikipedia.org/wiki/List_of_CJK_fonts

helm-buffer-list-with-multi-byte-string-03

thierryvolpiatto added a commit that referenced this issue Dec 14, 2012

* helm-buffers.el (helm-c-highlight-buffers): Issue #170 Revert again.
Keep using only `helm-substring-by-width' for all.
This is not working with european fonts AND using multibyte strings as
buffer names, but works fine with common Japonese/chinese fonts, so keep
it like this for now.
* helm-utils.el (helm-substring-by-width): Revert.
Owner

thierryvolpiatto commented Dec 14, 2012

Ok thanks for checking.
My english is not very good too :-)

We will keep what is working for you, it doesn't matter if it don't align perfectly on my side as people
have rarely such buffers in our zone.
I guess to fix that fully we should check the width of each char and do a more clever calculus of the number of space to add.

So I have reverted, please check again if it's ok.

Thanks again for your help on this.

Contributor

luozengbin commented Dec 15, 2012

Thanks a lot.

@luozengbin luozengbin closed this Dec 15, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment