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

Integrated markdown parsing and guide support #523

Merged
merged 1 commit into from
Jun 1, 2016
Merged

Integrated markdown parsing and guide support #523

merged 1 commit into from
Jun 1, 2016

Conversation

agentk
Copy link
Collaborator

@agentk agentk commented Apr 13, 2016

Fixes: #435
Rebases: #437

Adds two configuration options:

--documentation: A glob for extra documentation to be injected similar to readmes, but also listed in the sidebar.
--abstract: A glob for injecting extra content into matching Abstract group sections.

This is a rough commit. I have no idea what to test in ruby. Would love some suggestions on what to look at next,

@agentk
Copy link
Collaborator Author

agentk commented Apr 13, 2016

I think I also need to separate this into smaller testable chunks to be submitted separately as PR's.

@jpsim
Copy link
Collaborator

jpsim commented Apr 14, 2016

Integration specs are failing with the following stack trace:

ROOT/lib/jazzy/source_document.rb:32:in `block in fallback_readme': undefined local variable or method `config' for #<Jazzy::SourceDocument:0x007fdcd0d857c0> (NameError)
 from ROOT/lib/jazzy/source_document.rb:31:in `each'
 from ROOT/lib/jazzy/source_document.rb:31:in `fallback_readme'
 from ROOT/lib/jazzy/source_document.rb:23:in `readme_content'
 from ROOT/lib/jazzy/source_document.rb:18:in `content'
 from ROOT/lib/jazzy/doc_builder.rb:249:in `document_markdown'
 from ROOT/lib/jazzy/doc_builder.rb:354:in `document'
 from ROOT/lib/jazzy/doc_builder.rb:83:in `block (2 levels) in build_docs'
 from ROOT/lib/jazzy/doc_builder.rb:82:in `open'
 from ROOT/lib/jazzy/doc_builder.rb:82:in `open'
 from ROOT/lib/jazzy/doc_builder.rb:82:in `block in build_docs'
 from ROOT/lib/jazzy/doc_builder.rb:93:in `call'
 from ROOT/lib/jazzy/doc_builder.rb:93:in `block in each_doc'
 from ROOT/lib/jazzy/doc_builder.rb:89:in `each'
 from ROOT/lib/jazzy/doc_builder.rb:89:in `each_doc'
 from ROOT/lib/jazzy/doc_builder.rb:78:in `build_docs'
 from ROOT/lib/jazzy/doc_builder.rb:118:in `build_site'
 from ROOT/lib/jazzy/doc_builder.rb:146:in `build_docs_for_sourcekitten_output'
 from ROOT/lib/jazzy/doc_builder.rb:69:in `build'
 from ROOT/bin/jazzy:15:in `<main>'

@agentk
Copy link
Collaborator Author

agentk commented Apr 14, 2016

Yeah 😳. I missed a couple of things in the process of rebasing. This PR could do with a WIP label. I mainly wanted to show some progress to guilt myself into getting it finished.

@jpsim
Copy link
Collaborator

jpsim commented Apr 14, 2016

No guilt needed! Let me know if/how I can help.

@agentk
Copy link
Collaborator Author

agentk commented May 2, 2016

Woot. I'm finally rebased AND passing tests.

end

def self.documentation_entries
return [] unless
Copy link

Choose a reason for hiding this comment

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

Do you think it would make sense to filter files named index.* here too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I probably wouldn't hurt, but I'm not sure if it's necessary there. Without it, it makes it possible to override the readme and have it included in the Dash Guides section.

@jpsim
Copy link
Collaborator

jpsim commented May 3, 2016

It'd be good to add a basic test for this in the misc_jazzy_features fixture

Also, some basic documentation describing how to use this in jazzy's readme would be nice.

@agentk
Copy link
Collaborator Author

agentk commented May 3, 2016

Good catch. Will do. 👍

@jpsim jpsim mentioned this pull request May 13, 2016
@JanC
Copy link

JanC commented May 26, 2016

Hi,
did you have the chance to review this? It would be great to have this feature merged

@jpsim
Copy link
Collaborator

jpsim commented May 26, 2016

@JanC still waiting on:

  • basic test for this in the misc_jazzy_features fixture
  • basic documentation describing how to use this in jazzy's readme
  • merge conflicts being addressed

@agentk
Copy link
Collaborator Author

agentk commented May 28, 2016

Hey @jpsim, my first attempt at fixtures and Readme are in. The fixture tests are passing, but they are pointed at my fork until they are ok to merge in (realm/jazzy-integration-specs#18). Once they are the fixtures are right to merge, I'll point the submodule back to realm and we can merge this.

@jpsim
Copy link
Collaborator

jpsim commented Jun 1, 2016

This is super great! Love the feature, love the docs you added 😄

I've merged realm/jazzy-integration-specs#18 and given you commit access to jazzy repos to simplify this in the future.

I think the only thing this is missing now is a changelog entry. Could you make a new PR for that? I'll merge this one right away.

@jpsim jpsim merged commit b903d30 into realm:master Jun 1, 2016
@JanC
Copy link

JanC commented Jun 8, 2016

This was merged but not yet released, correct?

@agentk
Copy link
Collaborator Author

agentk commented Jun 8, 2016

Correct. Looks like it will probably go out with #569 :-)

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.

None yet

5 participants