Skip to content

Conversation

@BurdetteLamar
Copy link
Member

No description provided.

@BurdetteLamar BurdetteLamar requested a review from nobu September 28, 2025 01:53
@BurdetteLamar BurdetteLamar added the documentation Improvements or additions to documentation label Sep 28, 2025
- Fix accept_charset option description to clarify HTTP header vs config behavior
- Add note about max_multipart_length being internal-only (no public getter)
- Update pretty_inspect example to show accurate @accept_charset value
- Add 'Options vs Public Methods' section explaining internal vs external APIs
- Improve clarity around class default encoding vs method behavior

These changes address accuracy issues while preserving the excellent
structural improvements from the original enhanced documentation.
- Add test_cgi_new.rb with 11 tests covering all CGI.new functionality
- Test all documented options: accept_charset, max_multipart_length, tag_maker
- Validate all HTML versions: html3, html4, html4Tr, html4Fr, html5
- Test offline mode, encoding error handling, and option combinations
- Verify accept_charset method behavior (HTTP header vs configuration)
- Ensure proper integration with existing test suite
- 100% pass rate with comprehensive coverage of enhanced documentation

The test suite validates both the functionality and the accuracy of
the enhanced CGI.new documentation.
@ioquatix
Copy link
Member

@BurdetteLamar thanks for this work.

I evaluated the changes and found some of the documented behaviour was not aligned with the implementation. Do you mind reviewing my documentation changes? I also added tests (which test what the documentation asserts is true).

@ioquatix
Copy link
Member

FYI, it looks like the behaviour of pretty_inspect changed?

> ruby -Ilib -rcgi -rpp -e "cgi = CGI.new; puts cgi.pretty_inspect"
(offline mode: enter name=value pairs on standard input)
#<CGI:0x00000001220235c8
 @accept_charset=#<Encoding:UTF-8>,
 @accept_charset_error_block=nil,
 @cookies={},
 @max_multipart_length=134217728,
 @multipart=false,
 @options={accept_charset: #<Encoding:UTF-8>, max_multipart_length: 134217728},
 @output_cookies=nil,
 @output_hidden=nil,
 @params={}>

Previously:

> ruby -rcgi -rpp -e "cgi = CGI.new; puts cgi.pretty_inspect"
(offline mode: enter name=value pairs on standard input)
#<CGI:0x0000000100e255a8
 @accept_charset="UTF-8",
 @accept_charset_error_block=nil,
 @cookies={},
 @max_multipart_length=134217728,
 @multipart=false,
 @options={accept_charset: "UTF-8", max_multipart_length: 134217728},
 @output_cookies=nil,
 @output_hidden=nil,
 @params={}>

Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

With some minor changes. LGTM. Thanks.

@BurdetteLamar
Copy link
Member Author

Thanks, @ioquatix. I'm good with this change. Is the difference b/c I'm using an out-of-date CGI (0.5.-0, in Ruby version 3.4.5)?

@BurdetteLamar
Copy link
Member Author

@ioquatix, also, I don't like to merge my own work, so when you're satisfied, please feel free to merge.

@ioquatix ioquatix merged commit 5d50fbb into ruby:master Sep 29, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants