Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Add the option to generate HTML fragments and the body of an HTML doc… #1159

Closed
wants to merge 7 commits into from
Closed

Add the option to generate HTML fragments and the body of an HTML doc… #1159

wants to merge 7 commits into from

Conversation

rudkjobing
Copy link
Contributor

Implement feature requested here: #971 (comment)

@rudkjobing
Copy link
Contributor Author

rudkjobing commented Mar 11, 2017 •
edited
Loading

@fzaninotto Hmm is master broken? Tests wont run for me after I merged master into my fork.

Testing started at 10:18 PM ...
<?xml version="1.0" encoding="UTF-8"?><contacts>
PHP Notice:  Undefined offset: -32 in C:\SLFaker\src\Faker\Provider\Base.php on line 193
PHP Notice:  Undefined index:  in C:\SLFaker\src\Faker\Provider\Base.php on line 193
PHP Notice:  Undefined offset: -229 in C:\SLFaker\src\Faker\Provider\Base.php on line 193
PHP Notice:  Undefined index:  in C:\SLFaker\src\Faker\Provider\Base.php on line 193
PHP Fatal error:  Uncaught Exception: domainWord failed with the selected locale. Try a different locale or activate the "intl" PHP extension. in C:\SLFaker\src\Faker\Provider\Internet.php:138
Stack trace:
#0 C:\SLFaker\src\Faker\Provider\Internet.php(124): Faker\Provider\Internet->domainWord()
#1 [internal function]: Faker\Provider\Internet->domainName()
#2 C:\SLFaker\src\Faker\Generator.php(207): call_user_func_array(Array, Array)
#3 C:\SLFaker\src\Faker\Generator.php(241): Faker\Generator->format('domainName')
#4 [internal function]: Faker\Generator->callFormatWithMatches(Array)
#5 C:\SLFaker\src\Faker\Generator.php(236): preg_replace_callback('/\\{\\{\\s?(\\w+)\\s...', Array, '{{userName}}@{{...')
#6 C:\SLFaker\src\Faker\Provider\Internet.php(40): Faker\Generator->parse('{{userName}}@{{...')
#7 [internal function]: Faker\Provider\Internet->email()
#8 C:\SLFaker\src\Faker\Generator.php(207): call_user_func_array(Array, Array)
#9 C:\SLFaker\src\Faker\Generator.php(249): Faker\Generator->format('email')
#10 C:\SLFaker\test\te in C:\SLFaker\src\Faker\Provider\Internet.php on line 138
  <contact firstName="Arely" lastName="Champlin" email="
Notice: Undefined offset: -32 in C:\SLFaker\src\Faker\Provider\Base.php on line 193

Notice: Undefined index:  in C:\SLFaker\src\Faker\Provider\Base.php on line 193

Notice: Undefined offset: -229 in C:\SLFaker\src\Faker\Provider\Base.php on line 193

Notice: Undefined index:  in C:\SLFaker\src\Faker\Provider\Base.php on line 193

Fatal error: Uncaught Exception: domainWord failed with the selected locale. Try a different locale or activate the "intl" PHP extension. in C:\SLFaker\src\Faker\Provider\Internet.php:138
Stack trace:
#0 C:\SLFaker\src\Faker\Provider\Internet.php(124): Faker\Provider\Internet->domainWord()
#1 [internal function]: Faker\Provider\Internet->domainName()
#2 C:\SLFaker\src\Faker\Generator.php(207): call_user_func_array(Array, Array)
#3 C:\SLFaker\src\Faker\Generator.php(241): Faker\Generator->format('domainName')
#4 [internal function]: Faker\Generator->callFormatWithMatches(Array)
#5 C:\SLFaker\src\Faker\Generator.php(236): preg_replace_callback('/\\{\\{\\s?(\\w+)\\s...', Array, '{{userName}}@{{...')
#6 C:\SLFaker\src\Faker\Provider\Internet.php(40): Faker\Generator->parse('{{userName}}@{{...')
#7 [internal function]: Faker\Provider\Internet->email()
#8 C:\SLFaker\src\Faker\Generator.php(207): call_user_func_array(Array, Array)
#9 C:\SLFaker\src\Faker\Generator.php(249): Faker\Generator->format('email')
#10 C:\SLFaker\test\te in C:\SLFaker\src\Faker\Provider\Internet.php on line 138

Process finished with exit code 255

@Zhythero
Copy link

Sorry to be that guy, but any update on this?

WillGibson
WillGibson reviewed Jun 19, 2017
View reviewed changes
test/Faker/Provider/HtmlLoremTest.php Outdated
$faker = new Generator();
$faker->addProvider(new HtmlLorem($faker));
$node = $faker->randomBody(5, 5);
echo $node;
Copy link

Choose a reason for hiding this comment

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

I don't think you want the echo $node; line?

WillGibson
WillGibson reviewed Jun 19, 2017
View reviewed changes
test/Faker/Provider/HtmlLoremTest.php Outdated
$node = $faker->randomHtml(6, 10);
$node = $faker->randomHtml(5, 5);
$dom = new \DOMDocument();
$error = $dom->loadHTML($node);
Copy link

Choose a reason for hiding this comment

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

I think $isValidHTML would make more sense than $error as a variable name here and below.

@WillGibson
Copy link

Are, I was about to start making a a very similar PR. If you want any help getting this through the pip @Zhastreaus , give me a shout.

@Zhythero
Copy link

@WillGibson nope this is not my PR, I think it's @rudkjobing 's

@WillGibson
Copy link

@rudkjobing I'm going to fork from your fork and see if I can get the builds passing as I'd like to use this feature.

@rudkjobing
Copy link
Contributor Author

I must be blind Fatal error: Call to a member function randomNumber() on a non-object in /home/travis/build/fzaninotto/Faker/src/Faker/Provider/HtmlLorem.php on line 153

WillGibson
WillGibson reviewed Jun 20, 2017
View reviewed changes
test/Faker/Provider/HtmlLoremTest.php
@@ -20,10 +20,27 @@ public function testProvider()
public function testRandomHtmlReturnsValidHTMLString(){
$faker = new Generator();
$faker->addProvider(new HtmlLorem($faker));
$node = $faker->randomHtml(6, 10);
$node = $faker->randomHtml(5, 5);
$dom = new \DOMDocument();
$error = $dom->loadHTML($node);
Copy link

Choose a reason for hiding this comment

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

Maybe $isValidHtml instead of $error here too?

@WillGibson
Copy link

Looking good now @rudkjobing. Can't see any build info here now. Are they passing now you fixed the fatal error?

@WillGibson
Copy link

@rudkjobing I have a passing build here https://travis-ci.org/BiffBangPow/Faker/jobs/244895238. I'll try to submit a PR to your branch your with the fix.

@rudkjobing
Copy link
Contributor Author

Awesome, thanks!

@WillGibson
Copy link

rudkjobing#1

@rudkjobing
Merge pull request #1 from BiffBangPow/master
a1b8b76 
Fix indenting to make builds pass
@rudkjobing
Copy link
Contributor Author

Travis doesn't seem to run automatically?

@WillGibson
Copy link

It should run automatically because this is a PR I think, but you might need to log into Travis and add you repo/branch maybe?

@rudkjobing rudkjobing changed the base branch from master to country_names June 20, 2017 12:00
@rudkjobing rudkjobing changed the base branch from country_names to master June 20, 2017 12:00
@rudkjobing rudkjobing closed this Jun 20, 2017
@rudkjobing rudkjobing reopened this Jun 20, 2017
@rudkjobing rudkjobing closed this Jun 20, 2017
@rudkjobing rudkjobing reopened this Jun 20, 2017
@rudkjobing
Copy link
Contributor Author

I havn't had issues with travis before. Don't really know what the problem is.

@WillGibson
Copy link

Me neither, and I can't play with your Travis builds directly I guess. It should build automatically under fzaninotto/Faker as it's a PR, at least that what I thought. @fzaninotto Can you help here? I had a passing build here (https://travis-ci.org/BiffBangPow/Faker/builds/244900976) if that's any use.

WillGibson
WillGibson reviewed Jun 20, 2017
View reviewed changes
src/Faker/Provider/HtmlLorem.php Outdated
*
* @return string
*/
public function randomBody($maxDepth = 4, $maxWidth = 4)
Copy link

Choose a reason for hiding this comment

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

Now that I'm using this in my actual developing code, I think this would be better called randomHTMLBody.

@rudkjobing
Copy link
Contributor Author

Going to open a new pull request, to see if that helps.

@rudkjobing rudkjobing closed this Jun 20, 2017
@WillGibson
Copy link

Hi @rudkjobing

Fingers crossed re the new PR.

Please forgive my amatuerish approach to commenting on PRs in GitHub, I'm just getting used to how it all works (we use BitBucket here at work so the UI is different) and trying to get a bit more involved, help out with open source stuff.

There's a couple more comments in there from me that I would like addressed unless you disagree with them, then I'm totally going to approve this PR which has saved me doing the same thing over here.

https://github.com/fzaninotto/Faker/pull/1159/files#diff-ecbf0f117b3c8b63635081589370cce6R91

https://github.com/fzaninotto/Faker/pull/1159/files#diff-b3f77ef5414bc4a1b3641d7567ae3998R25

Will

@rudkjobing
Copy link
Contributor Author

rudkjobing commented Jun 20, 2017 •
edited
Loading

Hi Will.

Don't worry! I did change the code based on your change requests. I made a new PR here: #1237 but travis still ignores it.

@WillGibson WillGibson mentioned this pull request Jun 21, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@WillGibson WillGibson WillGibson left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@rudkjobing @Zhythero @WillGibson