Remove non-control characters from tokens before construction

Author: kputnam

ext/c/stupidedi/reader/native_ext.c

@@ -602,6 +602,83 @@ rb_min_graphic_index(int argc, const VALUE *argv, VALUE self)
     }
 }
 
+/*
+ * call-seq:
+ *   min_nongraphic_index(string, index=0) -> int
+ *
+ * Returns the smallest index (starting from the given index) that is not a
+ * graphic character. If no non-graphic characters occur after the given index,
+ * then the string length is returned.
+ *
+ *   min_nongraphic_index("\r\nabc ")     #=> 2
+ *   min_nongraphic_index("\r\nabc ", 2)  #=> 2
+ *   min_nongraphic_index("\r\nabc ", 5)  #=> 5
+ *   min_nongraphic_index("\r\n")         #=> 2
+ */
+static VALUE
+rb_min_nongraphic_index(int argc, const VALUE *argv, VALUE self)
+{
+    rb_check_arity(argc, 1, 2);
+
+    Check_Type(argv[0], T_STRING);
+    if (argc >= 2)
+        Check_Type(argv[1], T_FIXNUM);
+
+    VALUE str;
+    str = argv[0];
+
+    char *ptr, *end;
+    end = RSTRING_END(str);
+    ptr = RSTRING_PTR(str);
+
+    long idx;
+    idx = argc < 2 ? 0 : FIX2LONG(argv[1]);
+
+    int encidx;
+    encidx = ENCODING_GET(str);
+
+    rb_encoding *enc;
+    enc = rb_enc_from_index(encidx);
+
+    if (idx < 0)      rb_raise(rb_eArgError, "index cannot be negative");
+    if (ptr == NULL)  return INT2FIX(0);
+
+    if (single_byte_optimizable(str, enc)) {
+        ptr += idx; /* address of str[idx] */
+
+        if (end <= ptr)
+            return LONG2NUM(RSTRING_LEN(str));
+
+        while (ptr < end && is_graphic(*ptr, encidx))
+            ptr ++;
+
+        return LONG2NUM(ptr - RSTRING_PTR(str));
+    } else {
+      long len_, count;
+      len_  = 1;
+      count = 0;
+
+      /* address of str[idx], len is .bytesize */
+      ptr = rb_str_subpos(str, idx, &len_);
+      if (ptr == NULL) return rb_str_length(str);
+
+      unsigned int c;
+      int len;
+
+      while (ptr < end) {
+        c = rb_enc_codepoint_len(ptr, end, &len, enc);
+
+        if (!is_graphic(c, encidx))
+            break;
+
+        ptr   += len;
+        count ++;
+      }
+
+      return LONG2NUM(idx + count);
+    }
+}
+
 /*
  * call-seq:
  *   min_nonspace_index(string, index=0) -> int
@@ -771,6 +848,7 @@ void Init_native_ext(void) {
     rb_define_singleton_method(rb_m, "graphic?",            rb_graphic_p,           -1);
     rb_define_singleton_method(rb_m, "whitespace?",         rb_whitespace_p,        -1);
     rb_define_singleton_method(rb_m, "min_graphic_index",   rb_min_graphic_index,   -1);
+    rb_define_singleton_method(rb_m, "min_nongraphic_index",rb_min_nongraphic_index,-1);
     rb_define_singleton_method(rb_m, "min_nonspace_index",  rb_min_nonspace_index,  -1);
     rb_define_singleton_method(rb_m, "max_nonspace_index",  rb_max_nonspace_index,  -1);
 }

lib/stupidedi/reader.rb

@@ -230,7 +230,15 @@ class << self
 
       # @return [StreamReader]
       def build(input, *args)
-        Tokenizer.build(Input.build(input, *args))
+        if args.last.is_a?(Hash)
+          keywords = {}
+          keywords[:config] = args.last.delete(:config) if args.last.include?(:config)
+          keywords[:strict] = args.last.delete(:config) if args.last.include?(:strict)
+        else
+          keywords = {}
+        end
+
+        Tokenizer.build(Input.build(input, *args), *keywords)
       end
     end
   end

lib/stupidedi/reader/input.rb

@@ -15,7 +15,7 @@ class Input
       # @return [Position]
       attr_reader :position
 
-      def_delegators :@pointer, :head, :defined_at?, :empty?, :[],
+      def_delegators :@pointer, :head, :defined_at?, :empty?, :[], :to_s,
         :take, :index, :=~, :==, :at, :encoding, :offset, type: Substring
 
       def initialize(pointer, position)

lib/stupidedi/reader/substring.rb

@@ -260,8 +260,9 @@ def lstrip!(start_at = 0)
       def rstrip(start_at = @length - 1)
         return self if @length.zero?
         raise ArgumentError, "start_at must be non-negative" if start_at < 0
-        start_at  = @length - 1 if start_at >= @length
-        index     = NativeExt.max_nonspace_index(@storage, @offset + start_at)
+
+        start_at = @length - 1 if start_at >= @length
+        index    = NativeExt.max_nonspace_index(@storage, @offset + start_at)
 
         if index >= @offset + @length - 1
           self
@@ -315,6 +316,15 @@ def min_graphic_index(offset = 0)
         n - @offset
       end
 
+      def min_nongraphic_index(offset = 0)
+        raise ArgumentError, "offset must be non-negative" if offset < 0
+        offset = @length if offset > @length
+
+        n = NativeExt.min_nongraphic_index(@storage, @offset + offset)
+        n = @offset + @length if n > @offset + @length
+        n - @offset
+      end
+
       # Returns true if the character at the given offset is a control
       # character (defined by X222.pdf B.1.1.2.4 Control Characters)
       #
@@ -324,6 +334,30 @@ def graphic?(offset)
         NativeExt.graphic?(@storage, @offset + offset)
       end
 
+      # @private
+      # @return [String]
+      def clean
+        from = 0
+        upto = min_nongraphic_index
+        return self unless upto < @length
+
+        # We know Substring#<< cannot be zero-copy at this point
+        buffer = @storage[@offset + from, upto - from] unless from == upto
+
+        while from < @length and upto < @length
+          from = min_graphic_index(upto)
+          upto = min_nongraphic_index(from)
+
+          if buffer.nil?
+            buffer = @storage[@offset + from, upto - from]
+          else
+            buffer << @storage[@offset + from, upto - from]
+          end unless from == upto
+        end
+
+        buffer || ""
+      end
+
       # @endgroup
       #########################################################################
 

lib/stupidedi/reader/tokenizer.rb

@@ -282,14 +282,14 @@ def _next_segment_id(input)
         #
         # Later, the segment_id will be subject to many equality checks, so it
         # is faster overall to make the substring copy here.
-        segment_id = buffer.to_s
+        segment_id = buffer.clean.to_s
 
         return expected("segment identifier, found %s" % segment_id.inspect,
           start_pos) unless segment_id.match?(VALID_SEGMENT_ID)
 
-        # DEBUG
-        raise unless input.start_with?(@separators.segment, xx) \
-                  or input.start_with?(@separators.element, xx)
+        # TODO: DEBUG
+        # raise unless input.start_with?(@separators.segment, xx) \
+        #           or input.start_with?(@separators.element, xx)
 
         return done(segment_id.to_sym, start_pos, input.drop!(xx))
       end
@@ -328,8 +328,8 @@ def _read_elements(input, segment_id, element_uses)
           element_idx  += 1
         end
 
-        # DEBUG
-        raise unless input.start_with?(@separators.segment)
+        # TODO: DEBUG
+        # raise unless input.start_with?(@separators.segment)
 
         # Skip past the segment separator
         done(element_toks, nil, input.lstrip_nongraphic!(1))
@@ -415,18 +415,18 @@ def _read_component_element(input, repeatable, parent_repeatable, segment_id, el
             buffer = input[offset, length]
 
             if repeatable
-              builder.add(Tokens::SimpleElementTok.build(buffer, repeat_pos))
+              builder.add(Tokens::SimpleElementTok.build(buffer.clean, repeat_pos))
               offset     = rs + 1
               repeat_pos = input.position_at(offset)
             else # parent_repeatable
-              builder.add(Tokens::ComponentElementTok.build(buffer, repeat_pos))
+              builder.add(Tokens::ComponentElementTok.build(buffer.clean, repeat_pos))
               return done(builder.build, builder.position, input.drop!(rs))
             end
           elsif xx
             length = xx - offset
             buffer = input[offset, length]
 
-            builder.add(Tokens::ComponentElementTok.build(buffer, repeat_pos))
+            builder.add(Tokens::ComponentElementTok.build(buffer.clean, repeat_pos))
             return done(builder.build, builder.position, input.drop!(xx))
           else
             break
@@ -465,7 +465,7 @@ def _read_simple_element(input, repeatable, segment_id, element_idx)
             length = rs - offset
             buffer = input[offset, length]
 
-            builder.add(Tokens::SimpleElementTok.build(buffer, repeat_pos))
+            builder.add(Tokens::SimpleElementTok.build(buffer.clean, repeat_pos))
             offset     = rs + 1
             repeat_pos = input.position_at(offset)
           elsif xx
@@ -474,7 +474,7 @@ def _read_simple_element(input, repeatable, segment_id, element_idx)
             length = xx - offset
             buffer = input[offset, length]
 
-            builder.add(Tokens::SimpleElementTok.build(buffer, repeat_pos))
+            builder.add(Tokens::SimpleElementTok.build(buffer.clean, repeat_pos))
             return done(builder.build, start_pos, input.drop!(xx))
           else
             break

spec/lib/stupidedi/reader/substring_spec.rb

@@ -935,8 +935,8 @@ def matchp(num_calls)
   end
 
   describe "#rstrip" do
-    let(:sb) { Stupidedi::Reader::Pointer.build("  abc  ") }
-    let(:mb) { Stupidedi::Reader::Pointer.build("  💃🏽🕺🏻  ") }
+    let(:sb) { pointer("  abc  ") }
+    let(:mb) { pointer("  💃🏽🕺🏻  ") }
 
     context "when string is empty" do
       specify { expect(sb.take(0).rstrip).to eq("") }
@@ -955,15 +955,16 @@ def matchp(num_calls)
       end
 
       allocation do
+        sb; mb # pre-allocate Substring
         expect{ mb.rstrip }.to allocate(String: 0, mb.class => 1)
         expect{ sb.rstrip }.to allocate(String: 0, sb.class => 1)
       end
     end
   end
 
   describe "#lstrip" do
-    let(:sb) { Stupidedi::Reader::Pointer.build("  abc  ") }
-    let(:mb) { Stupidedi::Reader::Pointer.build("  💃🏽🕺🏻  ") }
+    let(:sb) { pointer("  abc  ") }
+    let(:mb) { pointer("  💃🏽🕺🏻  ") }
 
     context "when string is empty" do
       specify { expect(sb.take(0).lstrip).to eq("") }
@@ -982,6 +983,7 @@ def matchp(num_calls)
       end
 
       allocation do
+        sb; mb # pre-allocate Substring
         expect{ sb.lstrip }.to allocate(String: 0, sb.class => 1)
         expect{ mb.lstrip }.to allocate(String: 0, mb.class => 1)
       end
@@ -1015,4 +1017,34 @@ def matchp(num_calls)
 
   describe "#max_whitespace_index" do
   end
+
+  describe "#clean" do
+    context "when no control characters present" do
+      let(:sb) { pointer("*A^B~C:D*A:B~C*D^") }
+      let(:mb) { pointer("  💃🏽🕺🏻  ") }
+
+      allocation do
+        sb; mb # pre-allocate Substring
+        expect { sb.clean }.to allocate(String: 0)
+        expect { mb.clean }.to allocate(String: 0)
+      end
+
+      specify { expect(sb.clean).to eq(sb) }
+      specify { expect(mb.clean).to eq(mb) }
+    end
+
+    context "when some control characters present" do
+      let(:sb) { pointer("*A^B\r\nC:D*A:B\r\nC*D^") }
+      let(:mb) { pointer("\r\n💃🏽🕺🏻\r\n") }
+
+      allocation do
+        sb; mb # pre-allocate Substring
+        expect { sb.clean }.to allocate(String: 3)
+        expect { mb.clean }.to allocate(String: 1)
+      end
+
+      specify { expect(sb.clean).to eq(sb.storage.gsub(/[\r\n]/, "")) }
+      specify { expect(mb.clean).to eq(mb.storage.gsub(/[\r\n]/, "")) }
+    end
+  end
 end