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

Introduce support for keyword arguments #3323

Open
flavorjones opened this issue Nov 14, 2024 · 3 comments
Open

Introduce support for keyword arguments #3323

flavorjones opened this issue Nov 14, 2024 · 3 comments
Labels
event/hackday2024 RubyConf 2024 Hack Day

Comments

@flavorjones
Copy link
Member

flavorjones commented Nov 14, 2024

The Nokogiri API was largely designed and implemented for Ruby 1.9, which pre-dates keyword argument support and parameter forwarding with .... As a result, many methods are using positional arguments where keyword arguments are probably better.

For example, Nokogiri::XML::Document.parse is declared as:

def parse(string_or_io, url = nil, encoding = nil, options = ParseOptions::DEFAULT_XML)

I would like to update these method signatures to support keyword arguments without dropping support (yet) for positional arguments. In the case of Document.parse, this might look like:

def parse(string_or_io,
          url_ = nil, encoding_ = nil, options_ = XML::ParseOptions::DEFAULT_XML,
          url: url_, encoding: encoding_, options: options_)

I would also like to update the docstrings to describe the keyword arguments, and remove documentation for the positional arguments.

Another example is Nokogiri::XML() which is defined as:

def XML(thing, url = nil, encoding = nil, options = XML::ParseOptions::DEFAULT_XML, &block)
  Nokogiri::XML::Document.parse(thing, url, encoding, options, &block)
end

Methods that are forwarding positional arguments should be updated to more cleanly forward either keyword arguments or positional arguments, and avoid redeclaring parameter default values:

def XML(...)
  Nokogiri::XML::Document.parse(...)
end
@flavorjones
Copy link
Member Author

For an example of the kind of documentation I'd like to see, take a look at #3324 which updates Nokogiri::XML::Document.parse.

flavorjones added a commit that referenced this issue Nov 14, 2024
@flavorjones
Copy link
Member Author

The list of methods that need to be updated is at https://docs.google.com/spreadsheets/d/1pWM7N5D9073J8IRVO0yghIySTYhsJoO73sslAVxEO-A/edit?gid=0#gid=0

We'll work on this for RubyConf 2024 hack day!

flavorjones added a commit that referenced this issue Nov 14, 2024
**What problem is this PR intended to solve?**

Related to #3323, introducing keyword argument support in
`XML::Document.parse`.


**Have you included adequate test coverage?**

Some minor test coverage introduced.


**Does this change affect the behavior of either the C or the Java
implementations?**

n/a
@flavorjones flavorjones added the event/hackday2024 RubyConf 2024 Hack Day label Nov 14, 2024
openbl added a commit to openbl/nokogiri that referenced this issue Nov 14, 2024
openbl added a commit to openbl/nokogiri that referenced this issue Nov 14, 2024
openbl added a commit to openbl/nokogiri that referenced this issue Nov 14, 2024
@flavorjones
Copy link
Member Author

TODO: link back to this issue in doc strings for people to discover some documentation about the positional arguments

flavorjones added a commit that referenced this issue Nov 14, 2024
**What problem is this PR intended to solve?**

#3323

**Have you included adequate test coverage?**

No new behavior has been added, so existing tests are sufficient.

**Does this change affect the behavior of either the C or the Java
implementations?**

No.
openbl added a commit to openbl/nokogiri that referenced this issue Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
event/hackday2024 RubyConf 2024 Hack Day
Projects
None yet
Development

No branches or pull requests

1 participant