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.

Sign up for GitHub

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

Jump to bottom

Feature/load all #22

Merged
merged 7 commits into from
Sep 19, 2022
Merged

Feature/load all #22

merged 7 commits into from
Sep 19, 2022

Conversation

clumsyjedi
Copy link
Contributor

No description provided.

Frazer Irving added 2 commits May 23, 2021 20:45
test: adds a test for load-all?
e646968 
test the load-all? flag has no regression when called through both
parse-string and parse-stream
allow multiple yaml docs to be loaded from a single input
8ad1239 
respond to the load-all? flag by callin Yaml.loadAll in place of
Yaml.load
test: fix tests
4496ed7 
we need to recreate the stream for each test as it's consumed in the
process of testing
Frazer Irving and others added 2 commits July 18, 2021 20:43
fix: avoid duplication between load-str/load-stream
f2d68bb 
just dispatch to load-stream when we have a string, using a StringReader
@clumsyjedi
Merge branch 'master' into feature/load-all
9489e91
@borkdude
Copy link
Collaborator

@clumsyjedi Sorry we didn't get back to you. I've noticed this PR just now. Would you mind cleaning up the changes to project.clj and the .lein_repl_history file?/

/cc @lread

@lread
Copy link
Collaborator

lread commented Sep 16, 2022

Ah. Cool! For those, like me, who would find a description helpful:

A YAML 1.1 stream can contain multiple documents.
Currently, clj-yaml will only load the first document.
This change adds the ability to optionally load all documents.
Enable by specifying :load-all? true in opts for parse-string or parse-stream.

@clumsyjedi thanks for this PR.
We realize it's been a while, let me know if you want to finish up the minor changes we need before we can merge it.
If we don't hear from you by end of day September 18th, I'll just go ahead and assume we have your blessing.

lread
lread requested changes Sep 16, 2022
View reviewed changes
.lein-repl-history Outdated Show resolved Hide resolved
.nrepl-port Outdated Show resolved Hide resolved
project.clj Outdated Show resolved Hide resolved
@lread
Copy link
Collaborator

lread commented Sep 19, 2022

I will carry out recommended changes.

@lread lread merged commit 7254f61 into clj-commons:master Sep 19, 2022
borkdude pushed a commit that referenced this pull request Sep 23, 2022
@clumsyjedi @borkdude
Feature/load all (#22)
4627e70 
A YAML 1.1 stream can contain multiple documents.
Currently, clj-yaml will only load the first document.
This change adds the ability to optionally load all documents.
Enable by specifying `:load-all?` true in `opts` for `parse-string` or `parse-stream`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slipset slipset slipset left review comments

@lread lread lread requested changes

@marcomorain marcomorain Awaiting requested review from marcomorain

@borkdude borkdude Awaiting requested review from borkdude

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@clumsyjedi @borkdude @lread @slipset