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

Skip to load commented out words #7413

Merged
merged 4 commits into from
Jan 24, 2024
Merged

Skip to load commented out words #7413

merged 4 commits into from
Jan 24, 2024

Conversation

hsbt
Copy link
Member

@hsbt hsbt commented Jan 23, 2024

What was the end-user or developer problem that led to this PR?

Fixes #7393

The current YAML serializer couldn't handle commented out words.

What is your fix for the problem, implemented in this PR?

Skip to load after # symbol.

Make sure the following tasks are checked

@hsbt hsbt force-pushed the fix-yaml-comment branch 3 times, most recently from 37533bd to edefc95 Compare January 23, 2024 11:48
@hsbt hsbt force-pushed the fix-yaml-comment branch from edefc95 to 54b67fb Compare January 23, 2024 12:07
@williantenfen
Copy link
Contributor

williantenfen commented Jan 23, 2024

@hsbt sorry If I'm getting on your way... I'm just trying to contribute with something..

does it make sense to use a regex for this? or update the current ones?

I think I manage to pass some specs with something:

REMOVE_COMMENT_REGEX = /
      ^
      (["']?)  # optional opening quote
      (.*?)  # match for characters between quotes
      \1     # matching closing quote
      \s*    # Optional whitespace characters after the closing quote
      (?:\#(.*))?  # optional comment starting with '#'
      $
    /xo

  ...
  # and then call it when needed:


         if val == "[]" # empty array
              val = []
            else
              val = val.match(REMOVE_COMMENT_REGEX)[2]
            end

would pass this:

it "ignore comments" do
      yaml = <<~YAML
        ---
        Foo: "Lobster shouldn't go # should stay!" # test
        Bar: Something without quote! # another comment
        Mick: "Not... but crambs yes!"#test
        empty:
      YAML

      hash = {
        "Foo" => "Lobster shouldn't go # should stay!",
        "Bar" => "Something without quote!",
        "Mick" => "Not... but crambs yes!",
        "empty" => {},
      }

      expect(serializer.load(yaml)).to eq(hash)
    end

this would support # as regular text inside quotes..

I can try a PR as well, if you don't mind..

@hsbt hsbt merged commit 53604cb into master Jan 24, 2024
82 checks passed
@hsbt hsbt deleted the fix-yaml-comment branch January 24, 2024 09:11
@hsbt
Copy link
Member Author

hsbt commented Jan 24, 2024

@williantenfen Thanks for your support! I only focused configuration file for rubygems. If you interest to improve this PR, feel free to submit your idea and changes 👍

@@ -80,6 +84,14 @@ def load(str)
res
end

def strip_comment(val)
if val.include?("#") && !val.start_with?("#")
Copy link
Member

Choose a reason for hiding this comment

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

given this is operating on a new string, we can avoid the array & string allocations by using val.sub!(/#.*/, "")

deivid-rodriguez pushed a commit that referenced this pull request Feb 5, 2024
Skip to load commented out words

(cherry picked from commit 53604cb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YAML comments not respected when loading gemrc
3 participants