[ruby-core:101226] [Ruby master Bug#17364] Fix documentation for String#encode options

[ruby-core:101226] [Ruby master Bug#17364] Fix documentation for String#encode options

[tj]

Issue #17364 has been reported by tjschuck (T.J. Schuck).

----------------------------------------
Bug #17364: Fix documentation for String#encode options
https://bugs.ruby-lang.org/issues/17364

* Author: tjschuck (T.J. Schuck)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
(Everything below is written about `String#encode`, but it also applies to `String#encode!` — only referring to `encode` for brevity.)

The current signature for `String#encode` is `str.encode(dst_encoding, src_encoding [, options] )`, and the docs below say:

> The `options` Hash gives details for conversion...

However, starting in Ruby 2.7, if you pass the options as a Hash, you get the warning "Using the last argument as keyword parameters is deprecated", and in Ruby 3.0-preview1, it raises "ArgumentError (wrong number of arguments (given 3, expected 0..2))".

In the ultimately called `str_transcode`, you can see that the argument format is `"02:"` in [transcode.c:L2760](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/transcode.c#L2760), i.e. they're actually keyword arguments.

Since there are so many keyword arguments that don't have "obvious" defaults to document in the method signature itself, attached are two proposed patches for how to fix the docs:

1. `keep_options.diff` uses a similar approach to the one used in, e.g., `CSV#filter`, using `**options` in the signature and then specifying that those are intended to be keyword arguments — see [csv.rb:997-1042](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/lib/csv.rb#L997-L1042)

2. `list_all_kwargs.diff` is probably the more strictly "correct" option, but definitely more unwieldy.  You can see a similar usage to this in the current Ruby 2.7 docs for `CSV#new`, but even that has since changed to using the `**options` style — see [ruby/csv/pull/124](https://github.com/ruby/csv/pull/124)

---Files--------------------------------
keep_options.diff (1.85 KB)
list_all_kwargs.diff (2.44 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:101227] [Ruby master Bug#17364] Fix documentation for String#encode options

[merch-redmine]

Issue #17364 has been updated by jeremyevans0 (Jeremy Evans).


Which format should be used in documentation depends on the method implementation.  If the method ignores unrecognized keywords, then `**options` should be used.  If the method raises an ArgumentError for unrecognized keywords, then keywords should be listed explicitly.  This is to mirror how the method would work if implemented in Ruby.

In this case, `String#encode` ignores unrecognized keywords, so the `**options` approach is preferable:

```ruby
"a".encode('UTF-8', :foo=>:bar)
# => "a"
```

Note that you should use `str.encode(encoding, **options)`, (no `[]` around options), because a keyword splat is always optional.  Personally, I don't think we should use `[]` around optional arguments in call-seq, we should use a default value for the argument or separate call-seq line.  That is the approach recommended by the method documentation guide (doc/method_documentation.rdoc).

----------------------------------------
Bug #17364: Fix documentation for String#encode options
https://bugs.ruby-lang.org/issues/17364#change-88909

* Author: tjschuck (T.J. Schuck)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
(Everything below is written about `String#encode`, but it also applies to `String#encode!` — only referring to `encode` for brevity.)

The current signature for `String#encode` is `str.encode(dst_encoding, src_encoding [, options] )`, and the docs below say:

> The `options` Hash gives details for conversion...

However, starting in Ruby 2.7, if you pass the options as a Hash, you get the warning "Using the last argument as keyword parameters is deprecated", and in Ruby 3.0-preview1, it raises "ArgumentError (wrong number of arguments (given 3, expected 0..2))".

In the ultimately called `str_transcode`, you can see that the argument format is `"02:"` in [transcode.c:L2760](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/transcode.c#L2760), i.e. they're actually keyword arguments.

Since there are so many keyword arguments that don't have "obvious" defaults to document in the method signature itself, attached are two proposed patches for how to fix the docs:

1. `keep_options.diff` uses a similar approach to the one used in, e.g., `CSV#filter`, using `**options` in the signature and then specifying that those are intended to be keyword arguments — see [csv.rb:997-1042](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/lib/csv.rb#L997-L1042)

2. `list_all_kwargs.diff` is probably the more strictly "correct" option, but definitely more unwieldy.  You can see a similar usage to this in the current Ruby 2.7 docs for `CSV#new`, but even that has since changed to using the `**options` style — see [ruby/csv/pull/124](https://github.com/ruby/csv/pull/124)

---Files--------------------------------
keep_options.diff (1.85 KB)
list_all_kwargs.diff (2.44 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:101228] [Ruby master Bug#17364] Fix documentation for String#encode options

[tj]

Issue #17364 has been updated by tjschuck (T.J. Schuck).

File string_encode_docs.diff added

Aha, thanks for the pointer to `doc/method_documentation.rdoc`, Jeremy — very helpful!

Based on the info you provided, it seems like using `**options` and axing the `[]` in the call-seq is the preferred method.  The attached diff should be the "final" state, then.  Should I open a PR on GitHub since this is a "minor" change, submit a patch here, or just leave the diff below?

(It would also be ideal if this could be backported to 2.7 to save anyone else hitting the warning from taking the journey I did to get here.)

----------------------------------------
Bug #17364: Fix documentation for String#encode options
https://bugs.ruby-lang.org/issues/17364#change-88910

* Author: tjschuck (T.J. Schuck)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
(Everything below is written about `String#encode`, but it also applies to `String#encode!` — only referring to `encode` for brevity.)

The current signature for `String#encode` is `str.encode(dst_encoding, src_encoding [, options] )`, and the docs below say:

> The `options` Hash gives details for conversion...

However, starting in Ruby 2.7, if you pass the options as a Hash, you get the warning "Using the last argument as keyword parameters is deprecated", and in Ruby 3.0-preview1, it raises "ArgumentError (wrong number of arguments (given 3, expected 0..2))".

In the ultimately called `str_transcode`, you can see that the argument format is `"02:"` in [transcode.c:L2760](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/transcode.c#L2760), i.e. they're actually keyword arguments.

Since there are so many keyword arguments that don't have "obvious" defaults to document in the method signature itself, attached are two proposed patches for how to fix the docs:

1. `keep_options.diff` uses a similar approach to the one used in, e.g., `CSV#filter`, using `**options` in the signature and then specifying that those are intended to be keyword arguments — see [csv.rb:997-1042](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/lib/csv.rb#L997-L1042)

2. `list_all_kwargs.diff` is probably the more strictly "correct" option, but definitely more unwieldy.  You can see a similar usage to this in the current Ruby 2.7 docs for `CSV#new`, but even that has since changed to using the `**options` style — see [ruby/csv/pull/124](https://github.com/ruby/csv/pull/124)

---Files--------------------------------
keep_options.diff (1.85 KB)
list_all_kwargs.diff (2.44 KB)
string_encode_docs.diff (1.82 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:101229] [Ruby master Bug#17364] Fix documentation for String#encode options

[merch-redmine]

Issue #17364 has been updated by jeremyevans0 (Jeremy Evans).

Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED

tjschuck (T.J. Schuck) wrote in #note-2:
> Aha, thanks for the pointer to `doc/method_documentation.rdoc`, Jeremy — very helpful!
> 
> Based on the info you provided, it seems like using `**options` and axing the `[]` in the call-seq is the preferred method.  The attached diff should be the "final" state, then.  Should I open a PR on GitHub since this is a "minor" change, submit a patch here, or just leave the diff below?

I can apply your patch.

> (It would also be ideal if this could be backported to 2.7 to save anyone else hitting the warning from taking the journey I did to get here.)

Since you requested this, I'll mark this for backporting. I do not think it is worth it from a cost/benefit perspective to backport documentation patches, but whether to do so is up to branch maintainer.

----------------------------------------
Bug #17364: Fix documentation for String#encode options
https://bugs.ruby-lang.org/issues/17364#change-88911

* Author: tjschuck (T.J. Schuck)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED
----------------------------------------
(Everything below is written about `String#encode`, but it also applies to `String#encode!` — only referring to `encode` for brevity.)

The current signature for `String#encode` is `str.encode(dst_encoding, src_encoding [, options] )`, and the docs below say:

> The `options` Hash gives details for conversion...

However, starting in Ruby 2.7, if you pass the options as a Hash, you get the warning "Using the last argument as keyword parameters is deprecated", and in Ruby 3.0-preview1, it raises "ArgumentError (wrong number of arguments (given 3, expected 0..2))".

In the ultimately called `str_transcode`, you can see that the argument format is `"02:"` in [transcode.c:L2760](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/transcode.c#L2760), i.e. they're actually keyword arguments.

Since there are so many keyword arguments that don't have "obvious" defaults to document in the method signature itself, attached are two proposed patches for how to fix the docs:

1. `keep_options.diff` uses a similar approach to the one used in, e.g., `CSV#filter`, using `**options` in the signature and then specifying that those are intended to be keyword arguments — see [csv.rb:997-1042](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/lib/csv.rb#L997-L1042)

2. `list_all_kwargs.diff` is probably the more strictly "correct" option, but definitely more unwieldy.  You can see a similar usage to this in the current Ruby 2.7 docs for `CSV#new`, but even that has since changed to using the `**options` style — see [ruby/csv/pull/124](https://github.com/ruby/csv/pull/124)

---Files--------------------------------
keep_options.diff (1.85 KB)
list_all_kwargs.diff (2.44 KB)
string_encode_docs.diff (1.82 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:101230] [Ruby master Bug#17364] Fix documentation for String#encode options

[tj]

Issue #17364 has been updated by tjschuck (T.J. Schuck).


Thank you, Jeremy — I appreciate all the assistance you gave here!

----------------------------------------
Bug #17364: Fix documentation for String#encode options
https://bugs.ruby-lang.org/issues/17364#change-88912

* Author: tjschuck (T.J. Schuck)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED
----------------------------------------
(Everything below is written about `String#encode`, but it also applies to `String#encode!` — only referring to `encode` for brevity.)

The current signature for `String#encode` is `str.encode(dst_encoding, src_encoding [, options] )`, and the docs below say:

> The `options` Hash gives details for conversion...

However, starting in Ruby 2.7, if you pass the options as a Hash, you get the warning "Using the last argument as keyword parameters is deprecated", and in Ruby 3.0-preview1, it raises "ArgumentError (wrong number of arguments (given 3, expected 0..2))".

In the ultimately called `str_transcode`, you can see that the argument format is `"02:"` in [transcode.c:L2760](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/transcode.c#L2760), i.e. they're actually keyword arguments.

Since there are so many keyword arguments that don't have "obvious" defaults to document in the method signature itself, attached are two proposed patches for how to fix the docs:

1. `keep_options.diff` uses a similar approach to the one used in, e.g., `CSV#filter`, using `**options` in the signature and then specifying that those are intended to be keyword arguments — see [csv.rb:997-1042](https://github.com/ruby/ruby/blob/04b96fc322fe6547d2587f5ccbe0a835af93e48d/lib/csv.rb#L997-L1042)

2. `list_all_kwargs.diff` is probably the more strictly "correct" option, but definitely more unwieldy.  You can see a similar usage to this in the current Ruby 2.7 docs for `CSV#new`, but even that has since changed to using the `**options` style — see [ruby/csv/pull/124](https://github.com/ruby/csv/pull/124)

---Files--------------------------------
keep_options.diff (1.85 KB)
list_all_kwargs.diff (2.44 KB)
string_encode_docs.diff (1.82 KB)


-- 
https://bugs.ruby-lang.org/