Skip to content

Add example output after the code examples #82

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

Merged
merged 2 commits into from
Nov 18, 2015
Merged

Conversation

JoelMarcey
Copy link
Contributor

Fixes #47

@JoelMarcey
Copy link
Contributor Author

screenshot 2015-11-16 16 46 16

code_node = node.replace(code)
# If the example has HHVM output associated with it, then print it
# after the code example
if File.exists? (full_path + ".example.hhvm.out")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to include .expect too (but not .expectf etc)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, I always expect a .example.hhvm.out for output.

I instruct example creators to directly copy the .expect file as an .example.hhvm.out;

if there is an .expectf, then I should write that you run the the test with no-clean and copy the resulting .out file into .example.hhvm.out.

We could check for .hhvm.expect to optionally avoid the copy. Check for .example.hhvm.out, if doesn't exist, then check for .hhvm.expect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, avoiding the copy would be nice. It would also be good if we could check that the .example.hhvm.out match the expectf/expectre

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would also be good if we could check that the .example.hhvm.out match the expectf/expectre

By running the test runner in process here for that particular example? Or some sort of regex check? All of our examples should pass the test runner. So if you produce the .example.hhvm.out via a run of the test runner, they would have to match because the test pass.

@JoelMarcey
Copy link
Contributor Author

Maybe I don't understand git well enough :/

I just updated the pull request and pushed the changes. My changes are in a70ddfb, but why do I see those other two commits from @fredemmott there?

Are we in sync? Or do I need to do something else?

@JoelMarcey JoelMarcey self-assigned this Nov 17, 2015
@JoelMarcey
Copy link
Contributor Author

Heck, I think I know why. I probably rebased against master instead of this branch and pull in the latest commits from master. Ugh. Ok. I will try to fix.

@JoelMarcey
Copy link
Contributor Author

OK. I think we are settled now. Interactive rebase and force push.

output = doc.document.create_element(
'pre',
File.read(output_path),
language: 'Bash',
Copy link
Contributor

Choose a reason for hiding this comment

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

? Why not text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh whoops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wait - I did do that on purpose actually.

This is output like you would see at the shell, from the command line. That's why I chose Bash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change it to "text". That's fine too, I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think bash gives you shell-script highlighting - i.e. it's for files that start with #!/bin/sh or #!/bin/bash

JoelMarcey added a commit that referenced this pull request Nov 17, 2015
No need to duplicate the two files for output. Just use the `.hhvm.expect` file.

See: #82

I will update the CONTRIBUTING.md documentation to reflect this.
@JoelMarcey
Copy link
Contributor Author

I committed 49718bf in preparation for this. So the only .example.hhvm.out files that exist now are the ones where we have a .expectf file. We can add an option to the test runner called --keep-explicit-output that produces a hhvm.out file if there is a .expectf file.

But for now we have manually added them in the interim.

Any way to merge this before the push today?

JoelMarcey added a commit that referenced this pull request Nov 18, 2015
We are going to be automatically adding output files via #82
fredemmott added a commit that referenced this pull request Nov 18, 2015
Add example output after the code examples
@fredemmott fredemmott merged commit 8603d3a into master Nov 18, 2015
@JoelMarcey JoelMarcey deleted the add-example-output branch November 19, 2015 03:45
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.

3 participants