Fix issue #122 #123

2018-02-26T09:58:51+01:00

waddlaw commented

2018-02-26 09:58:51 +01:00

(Migrated from github.com)

My idea is replace length to elasticLength that it is calclate length is two if **non**-ascii characters.

Implement elasticLength like this:

-- |
-- >>> elasticLength "あ"
-- 2
-- >>> elasticLength "abc"
-- 3
-- >>> elasticLength "aあa"
-- 4
elasticLength :: Text -> Int
elasticLength = Text.foldl' (\len c -> if isAscii c then len + 1 else len + 2) 0

I will not break anything so far.
@lspitzner Could you merge this?

Ref: #122 My idea is replace `length` to `elasticLength` that it is calclate length is two if `**non**-ascii characters`. Implement `elasticLength` like this: ```haskell -- | -- >>> elasticLength "あ" -- 2 -- >>> elasticLength "abc" -- 3 -- >>> elasticLength "aあa" -- 4 elasticLength :: Text -> Int elasticLength = Text.foldl' (\len c -> if isAscii c then len + 1 else len + 2) 0 ``` I will not break anything so far. @lspitzner Could you merge this?

pythonissam commented

2018-03-05 05:30:21 +01:00

(Migrated from github.com)

@lspitzner This PR seems to have fixed the issue. Would you consider merging it?

lspitzner commented

2018-03-05 14:45:07 +01:00

(Migrated from github.com)

Thanks for this contribution!

What is the additional dependency for?
This does not work for me. With my font, the those glyphs do not have a width of 2 ascii glyphs, so this does not give proper alignment still. Just pointing this out; If it works for you and other users I can merge still.
Please test this with some other alphabets/codepages and report what you tested (I don't want an additional cabal testsuite or anything, but some example that I can test manually). I highly suspect that stuff will break, because isAscii is a rather poor estimation of glyph size in general. And any unicode stuff should be tested with more than just two languages.

Thanks for this contribution! 1. What is the additional dependency for? 2. This does not work for me. With my font, the those glyphs do not have a width of 2 ascii glyphs, so this does not give proper alignment still. Just pointing this out; If it works for you and other users I can merge still. 3. Please test this with some other alphabets/codepages and report what you tested (I don't want an additional cabal testsuite or anything, but some example that I can test manually). I highly suspect that stuff will break, because isAscii is a rather poor estimation of glyph size in general. And any unicode stuff should be tested with more than just two languages.

lspitzner commented

2018-04-08 21:39:54 +02:00

(Migrated from github.com)

Closing due to inactivity, and because afaict this change makes things worse for many other languages (e.g. consider greek or russian).

waddlaw commented

2018-04-09 03:13:00 +02:00

(Migrated from github.com)

@lspitzner I understand. Thank you!

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#123