-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
JSON Output for Documentation Data #2777
Conversation
CI wants you to run |
|
||
opts.on("-h", "--help", "Show this message") do | ||
puts opts | ||
exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we do that on other commands? I don't think it's very common, to fail the help command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had copied it from, if I recall correctly, the build command, I don't have an opinion either way
:name => name, | ||
:value => value.to_s, | ||
:doc => doc | ||
}.to_json(io) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably use named tuples or io.json_object to avoid allocating extra objects (should consume less memory and be a bit faster, though I don't know if it'll be noticeable)
c11ddba
to
4ecc73c
Compare
when "json" | ||
generate_json_docs types | ||
else | ||
raise "Unknown format specified: #{@format}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given it's a user oriented message, this should abort
rather than raise
.
included_modules: included_modules_name_array, | ||
extended_modules: extended_modules_name_array, | ||
subclasses: subclasses_full_name_array, | ||
including_types: including_types_name_array, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need these methods. I don't like the array
in their name (bad for the same reasons that hungarian notation is bad), and included_modules.map(&.full_name)
is short enough.
I think that this is getting really close to merging. I'll keep tweaking with it a little, but please tell me if you need/want me to change anything. |
output_dir = dir | ||
end | ||
|
||
opts.on("-f html|json", "--format text|json", "Output format HTML (default) or HTML") do |f| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or JSON
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😆 Thanks. I would've missed that.
doc: doc, | ||
prefix: prefix, | ||
abstract: abstract?, | ||
kind: kind, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if kind
being stuff like def self.
isn't very machine friendly.
6443496
to
c7d9a7f
Compare
def to_json(io) | ||
{ | ||
kind: kind, | ||
name: name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this should this be empty for "Top Level Namespace"?
This can be closed. #4746 has been merged with JSON output. |
Fixes #2772