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

bug fix for sub/2 #586

Closed
pkoppstein opened this issue Sep 27, 2014 · 7 comments
Closed

bug fix for sub/2 #586

pkoppstein opened this issue Sep 27, 2014 · 7 comments
Labels
Milestone

Comments

@pkoppstein
Copy link
Contributor

There's a bug in builtin.c:sub -- a length check is missing. The corrected code is below. A relevant test case is:

"abc" | sub("^ +"; "X")
# If s contains capture variables, then create a capture object and pipe it to s
def sub(re; s):
  . as $in
  | [match(re)]
  | if length == 0 then $in
    else .[0]
    | . as $r
    # create the "capture" object:
    | reduce ( $r | .captures | .[] | select(.name != null) | { (.name) : .string } ) as $pair
        ({}; . + $pair) 
    | if . == {} then $in | .[0:$r.offset]+s+.[$r.offset+$r.length:]
      else (. | s)
      end
    end ;
@pkoppstein
Copy link
Contributor Author

@nicowilliams - Now that you're back, could you please install the update?

#576 also requires your attention, somewhat urgently in my opinion, as "split" is often essential. Thanks.

@nicowilliams
Copy link
Contributor

@pkoppstein This sub/2 fails to compile.

@pkoppstein
Copy link
Contributor Author

@nicowilliams - It loads fine here, and the following builtin.c variant (with $re) compiles here as well. Perhaps you have a "$" or ";" or some-such out of place? In any case, the only needed change is the "length" check, so you may find it simplest to roll your own.

  // If s contains capture variables, then create a capture object and pipe it to s
  "def sub($re; s):"
  "  . as $in"
  "  | [match($re)]"
  "  | .[0]"
  "  | . as $r"
     //  # create the \"capture\" object:
  "  | reduce ( $r | .captures | .[] | select(.name != null) | { (.name) : .string } ) as $pair"
  "      ({}; . + $pair)"
  "  | if . == {} then $in | .[0:$r.offset]+s+.[$r.offset+$r.length:]"
  "    else (. | s)"
  "    end ;",

@pkoppstein
Copy link
Contributor Author

#594 installs this fix, and makes trivial modifications to add gsub/3. The resultant jq has been tested.

@nicowilliams
Copy link
Contributor

Thanks! That's much more convenient. I'm at an airport, so connectivity
spotty, but i'll get to this by tomorrow night unless @wtlangford gets to
it sooner.

@wtlangford
Copy link
Contributor

I'll snag it tonight.
On Oct 6, 2014 3:54 PM, "Nico Williams" notifications@github.com wrote:

Thanks! That's much more convenient. I'm at an airport, so connectivity
spotty, but i'll get to this by tomorrow night unless @wtlangford gets to
it sooner.


Reply to this email directly or view it on GitHub
#586 (comment).

wtlangford pushed a commit to wtlangford/jq that referenced this issue Oct 7, 2014
Signed-off-by: William Langford <wlangfor@gmail.com>
@wtlangford
Copy link
Contributor

Closed with 85f0e30 (Why does this only auto-close half of the time?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants