Skip to content

xpath abbreviate: add support for string literal that contains double-quote - #96

Merged
kou merged 3 commits into
ruby:masterfrom
pulver:master
May 26, 2023
Merged

xpath abbreviate: add support for string literal that contains double-quote#96
kou merged 3 commits into
ruby:masterfrom
pulver:master

Conversation

@pulver

@pulver pulver commented May 25, 2023

Copy link
Copy Markdown
Contributor

This adds support for a string literal that contains a double-quote to XPathParser#abbreviate. Basically any literal that contains a double-quote " must be quoted by single-quotes ' since XPath 1.0 does not support any escape characters.

The change improves the following test script

require 'rexml'

parsed = REXML::Parsers::XPathParser.new.parse('/a[b/text()=concat("c\'",\'"d\')]')
puts "#{parsed}"
puts ""
appreviated = REXML::Parsers::XPathParser.new.abbreviate parsed
puts "#{appreviated}"

Output Before Change

[:document, :child, :qname, "", "a", :predicate, [:eq, [:child, :qname, "", "b", :child, :text], [:function, "concat", [[:literal, "c'"], [:literal, "\"d"]]]]]

/a[ b/text() = concat( "c'" , "\"d" ) ]

Output After Change

[:document, :child, :qname, "", "a", :predicate, [:eq, [:child, :qname, "", "b", :child, :text], [:function, "concat", [[:literal, "c'"], [:literal, "\"d"]]]]]

/a[ b/text() = concat( "c'" , '"d' ) ]

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this work with a number literal?

@pulver

pulver commented May 25, 2023

Copy link
Copy Markdown
Contributor Author

Good catch, thank you. I restored the previous method of literal.inspect for all non-Strings.

I wasn't sure how to handle this literal translation so I left it as it was:

when :literal
string << %Q{ "#{path.shift}" }

Comment thread lib/rexml/parsers/xpathparser.rb Outdated
path
end

def QuoteLiteral literal

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you use snake_case?

Suggested change
def QuoteLiteral literal
def quote_literal(literal)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And could you move this method to before #LocalPath (the line 192)?
Method in #LocationPath to #FunctionCall are for syntaxes in XPath. I don't want to mix this and them.

Comment thread lib/rexml/parsers/xpathparser.rb Outdated
def QuoteLiteral literal
case literal
when String
# Xpath 1.0 does not support escape characters.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# Xpath 1.0 does not support escape characters.
# XPath 1.0 does not support escape characters.
Comment thread lib/rexml/parsers/xpathparser.rb Outdated
Comment on lines +371 to +372
pattern = literal.include?('"') ? "'%s'" : '"%s"'
pattern % literal

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you use string interpolation instead of String#%?

Suggested change
pattern = literal.include?('"') ? "'%s'" : '"%s"'
pattern % literal
if literal.include?("'")
"\"#{literal}\""
else
"'#{literal}'"
end
 * Rename QuoteLiteral -> quote_literal and move to after predicate_to_string.
 * Xpath -> XPath.
 * Replace %-substitution w/ String "#{...}".
@pulver

pulver commented May 26, 2023

Copy link
Copy Markdown
Contributor Author

Done, thanks for the feedback.

Comment thread lib/rexml/parsers/xpathparser.rb Outdated
end
end

private

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you move this private above quote_literal?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done: d6a02b9

@kou kou changed the title Add XPathParser#QuoteLiteral. May 26, 2023
@kou kou changed the title xpath abbreviate: add support for string literal that includes double quote May 26, 2023
@kou
kou merged commit e08c52f into ruby:master May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants