Subject: [sup-talk] display_length issue with special-characters on non-UTF8 terminal

From: [email-masked] (Tarko Tikan)
Date: Tue, 09 Jun 2009 13:00:13 +0300

hey,

When String.display_length was introduced in recent update, it broke the length for non-UTF8 strings that contain the special characters. Wrong length results corrupted display (line ends chopped off).

Terminal is iso-8859-15 and it's detected by sup correctly.

I've tracked it down to /./u regexp. Here are some examples:

irb(main):001:0> "asd".scan(/./u)
=> ["a", "s", "d"]
irb(main):002:0> "asd????".scan(/./u)
=> ["a", "s", "d", "\365\374\344\366"]
irb(main):017:0> "asd???".scan(/./u)
=> ["a", "s", "d"]

irb(main):008:0* "asd".scan(/./)
=> ["a", "s", "d"]
irb(main):009:0> "asd????".scan(/./)
=> ["a", "s", "d", "\365", "\374", "\344", "\366"]

Expecting UTF8 gives unexpected results :) Also, old behaviour of String.length gives correct results with these test cases.

-- 
tarko

---

From: [email-masked] (William Morgan)
Date: Fri, 12 Jun 2009 12:18:11 -0700

Reformatted excerpts from Tarko Tikan's message of 2009-06-09:
> When String.display_length was introduced in recent update, it broke
> the length for non-UTF8 strings that contain the special characters.
> Wrong length results corrupted display (line ends chopped off).

That's a good point. I got a little utf8-centric with those changes.
(I'm assuming that your terminal encoding is not UTF-8.)

Does this patch fix the issue? If so, I will release an 0.8.1.

--- cut here ---
diff --git a/lib/sup.rb b/lib/sup.rb
index 4f59eaa..20835ae 100644
--- a/lib/sup.rb
+++ b/lib/sup.rb
@@ -244,7 +244,7 @@ end
     Redwood::log "using character set encoding #{$encoding.inspect}"
   else
     Redwood::log "warning: can't find character set by using locale, defaulting
-    $encoding = "utf-8"
+    $encoding = "UTF-8"
   end
 
 ## now everything else (which can feel free to call Redwood::log at load time)
diff --git a/lib/sup/util.rb b/lib/sup/util.rb
index 8a3004f..d5310bc 100644
--- a/lib/sup/util.rb
+++ b/lib/sup/util.rb
@@ -172,7 +172,13 @@ class Object
 end
 
 class String
-  def display_length; scan(/./u).size end
+  def display_length
+    if $encoding == "UTF-8"
+      scan(/./u).size
+    else
+      size
+    end
+  end
 
   def camel_to_hyphy
     self.gsub(/([a-z])([A-Z0-9])/, '\1-\2').downcase

-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Tarko Tikan)
Date: Sat, 13 Jun 2009 14:13:06 +0300

> (I'm assuming that your terminal encoding is not UTF-8.)

No, it's not.

> Does this patch fix the issue? If so, I will release an 0.8.1.

Yes it does. To me, this approach felt "hackish" so I didn't come up with a patch :) But I still don't have better idea how to fix it so it'll have to stay like this.

> +    if $encoding == "UTF-8"

It would probably be correct to use:

if $encoding == "UTF-8"
	scan(/./u).size
else
	length
end

Thats because scan returns a array (hence using the size), without scan you are just invoking on string and it's correct to use length (for some reason size works too, backward compatibility?) 

-- 
tarko

---

From: [email-masked] (William Morgan)
Date: Mon, 15 Jun 2009 07:10:30 -0700

Reformatted excerpts from Tarko Tikan's message of 2009-06-13:
> Yes it does. To me, this approach felt "hackish" so I didn't come up
> with a patch :) But I still don't have better idea how to fix it so
> it'll have to stay like this.

It's hackish because Ruby 1.8 has shitty multibyte support. The only
reason it works at all is because byte length is character length (at
least most of the time) in your encoding.

There is a multibyte gem out there that I'm keeping an eye on. Also Ruby
1.9.1 allegedgly fixes this problem.

> Thats because scan returns a array (hence using the size), without
> scan you are just invoking on string and it's correct to use length
> (for some reason size works too, backward compatibility?) 

Size and length are synonmys for both arrays and strings. I used size
there for symmetry.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Wed, 17 Jun 2009 09:04:34 -0700

Reformatted excerpts from Tarko Tikan's message of 2009-06-13:
> william wrote:
> Yes it does.  patch :) But I still don't have better idea how to fix
> it so it'll have to stay like this.

I have released an 0.8.1 which has this patch in it.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Nicolas Pouillard)
Date: Wed, 17 Jun 2009 20:34:08 +0200

> I have released an 0.8.1 which has this patch in it.

I still have issues with display_length. I use UTF-8, urxvt
and some characters disappear when a line contains special characters.

For instance in thread-view-mode if a line contains a special character
then the last character is dropped.

I've "fixed" the issue by reverting a display_length call to a size call
as in the attached patch.

diff --git a/lib/sup/buffer.rb b/lib/sup/buffer.rb
index 8eedf96..795b4c9 100644
--- a/lib/sup/buffer.rb
+++ b/lib/sup/buffer.rb
@@ -114,7 +114,7 @@ class Buffer
     stringl += 1 while stringl < s.length && s[0 ... stringl].display_length < maxl
     @w.mvaddstr y, x, s[0 ... stringl]
     unless opts[:no_fill]
-      l = s.display_length
+      l = s.size
       unless l >= maxl
         @w.mvaddstr(y, x + l, " " * (maxl - l))
       end

-- 
Nicolas Pouillard
http://nicolaspouillard.fr