Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text.to_display_text is (shortened) identity #6174

Merged
merged 7 commits into from
Apr 5, 2023
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.enso.interpreter.node.expression.builtin.text;

import com.ibm.icu.text.BreakIterator;
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.Specialization;
Expand All @@ -11,6 +13,7 @@
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.node.expression.builtin.text.util.TypeToDisplayTextNode;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.polyglot.common_utils.Core_Text_Utils;

@BuiltinMethod(type = "Any", name = "to_display_text")
public abstract class AnyToDisplayTextNode extends Node {
Expand All @@ -32,6 +35,22 @@ Text showExceptions(
}
}

@Specialization
Text convertText(Text self) {
final var limit = 80;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't that be lifted to a (private static final) constant ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally the limit was only used locally in a single method. Then I had to move the iterator manipulation behind @TruffleBoundary...

if (self.length() < limit) {
return self;
} else {
return takePrefix(self, limit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we an ellipsis here (U+2026)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea indeed.

Copy link
Member Author

@JaroslavTulach JaroslavTulach Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we an ellipsis here

"should we add an ellipsis?" is the question?

}
}

@CompilerDirectives.TruffleBoundary
private static Text takePrefix(Text self, final int limit) {
var prefix = Core_Text_Utils.take_prefix(self.toString(), limit);
return Text.create(prefix);
}

@Fallback
Text doShowType(Object self, @Cached TypeToDisplayTextNode typeToDisplayTextNode) {
return Text.create(typeToDisplayTextNode.execute(self));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import com.ibm.icu.text.BreakIterator;

public class Core_Text_Utils {
private Core_Text_Utils() {
}

/** Computes the length of the string as the number of grapheme clusters it contains. */
public static int computeGraphemeLength(String text) {
BreakIterator iter = BreakIterator.getCharacterInstance();
Expand All @@ -14,6 +17,17 @@ public static int computeGraphemeLength(String text) {
return len;
}

/** Returns a prefix of the string not exceeding the provided grapheme length. */
public static String take_prefix(String str, long grapheme_length) {
BreakIterator iter = BreakIterator.getCharacterInstance();
iter.setText(str);
if (iter.next(Math.toIntExact(grapheme_length)) == BreakIterator.DONE) {
return str;
} else {
return str.substring(0, iter.current());
}
}

/** Pretty prints the string, escaping special characters. */
public static String prettyPrint(String str) {
int len = str.length();
Expand Down
8 changes: 1 addition & 7 deletions std-bits/base/src/main/java/org/enso/base/Text_Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,7 @@ public static long grapheme_length(String str) {

/** Returns a prefix of the string not exceeding the provided grapheme length. */
public static String take_prefix(String str, long grapheme_length) {
BreakIterator iter = BreakIterator.getCharacterInstance();
iter.setText(str);
if (iter.next(Math.toIntExact(grapheme_length)) == BreakIterator.DONE) {
return str;
} else {
return str.substring(0, iter.current());
}
return Core_Text_Utils.take_prefix(str, grapheme_length);
}

/** Returns a suffix of the string not exceeding the provided grapheme length. */
Expand Down
23 changes: 23 additions & 0 deletions test/Tests/src/Data/Text/Utils_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,27 @@ spec =
Text_Utils.take_suffix (kshi+kshi+'a'+kshi) 2 . should_equal 'a'+kshi
Text_Utils.take_suffix (kshi+kshi+'a'+kshi) 1 . should_equal kshi

Test.group "to_display_text" <|
Test.specify "simple conversion" <|
"Hello".to_display_text . should_equal "Hello"

Test.specify "long text conversion" <|
long = "Hello World! ".repeat 1024
disp = long.to_display_text
disp.length . should_equal 80
disp.characters.take (First 5) . should_equal [ 'H', 'e', 'l', 'l', 'o' ]
disp.characters.take (Last 6) . should_equal ['l', 'd', '!', ' ', 'H', 'e']

Test.specify "grapheme 1 conversion" <|
txt = 'a\u0321\u0302'*100
txt.to_display_text . should_equal 'a\u0321\u0302'*80

Test.specify "grapheme 2 conversion" <|
txt = '\u0915\u094D\u0937\u093F'*100
txt.to_display_text . should_equal '\u0915\u094D\u0937\u093F'*80

Test.specify "grapheme 3 conversion" <|
txt = '\u{1F926}\u{1F3FC}\u200D\u2642\uFE0F'*100
txt.to_display_text . should_equal '\u{1F926}\u{1F3FC}\u200D\u2642\uFE0F'*80

main = Test_Suite.run_main spec