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

Potential Typo in REXML::Functions::normalize_space() Implementation #110

Closed
flatland001 opened this issue Feb 7, 2024 · 1 comment · Fixed by #111
Closed

Potential Typo in REXML::Functions::normalize_space() Implementation #110

flatland001 opened this issue Feb 7, 2024 · 1 comment · Fixed by #111

Comments

@flatland001
Copy link
Contributor

Hello Ruby/REXML developers,

The implementation can be found here:

https://github.com/ruby/rexml/blob/7e4049f6a68c99c4efec2df117057ee080680c9f/lib/rexml/functions.rb#L265C1-L273C8

The current code is as follows:

    # UNTESTED
    def Functions::normalize_space( string=nil )
      string = string(@@context[:node]) if string.nil?
      if string.kind_of? Array
        string.collect{|x| string.to_s.strip.gsub(/\s+/um, ' ') if string}
      else
        string.to_s.strip.gsub(/\s+/um, ' ')
      end
    end

It appears there may be a typo in the use of x versus string within the collect block. I believe the following adjustment might be more in line with the intended functionality:

string.collect{|x| x.to_s.strip.gsub(/\s+/um, ' ') if x}

I might be misunderstanding the intention here, so if I've overlooked something, please do let me know.
Thank you for your time.

@kou
Copy link
Member

kou commented Feb 7, 2024

Good catch!
Could you open a PR for this?
(It's better that it has a test for this case.)

flatland001 added a commit to flatland001/rexml that referenced this issue Feb 7, 2024
Replaced the variable 'string' with 'x' inside the collect block.
flatland001 added a commit to flatland001/rexml that referenced this issue Feb 7, 2024
…y#110

to validate `REXML::Functions::normalize_space()` functionality
flatland001 added a commit to flatland001/rexml that referenced this issue Feb 7, 2024
flatland001 added a commit to flatland001/rexml that referenced this issue Feb 8, 2024
Renamed test_normalize_space2 for better clarity and updated tests to use Array of String for expected data. ruby#110

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@kou kou closed this as completed in #111 Feb 8, 2024
kou added a commit that referenced this issue Feb 8, 2024
GitHub: fix GH-110

Fixed a bug in `REXML::Functions.normalize_space(array)` and introduced
test cases for it:

- Corrected a typo in the variable name within the collect block
(`string` -> `x`).
- Added `test_normalize_space_strings` to `test/functions/test_base.rb`.

---------

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants