73rhodes / sideflow

Flow control extension for Selenium IDE
MIT License
136 stars 209 forks source link

I've added For/Each looping to your sideFlow plugin for Selenium. #3

Closed martinhbramwell closed 12 years ago

martinhbramwell commented 12 years ago

Hi,

I hope you like what I've done to your code. I don't think I've busted anything, but it's always possible.

It's my first attempt at customizing Selenium so there are certainly nice things you could do to it, if you are so inclined

Sincere regards, Martin "Hasan" Bramwell


Added my enhancement code to sideflow.js Added my enhancement documentation to README.md Created ./demos/testForEach.html to demonstrate how the enhancement works. Included Darren DeRidder's ./demos/test.html AND ./demos/testForEach.html in a new test suite -- SuiteSIDEFlow.html

darrenderidder commented 12 years ago

Hi Martin, thanks for this. Your diff on sideflow.js file is actually a replacement of the entire contents of the file. Could you please submit your changes without modifying the other lines, so it's possible to see exactly what's been changed? That will make code review and testing a lot easier before pulling it in.
cheers, Darren

martinhbramwell commented 12 years ago

Yeah, that's really exasperating. It's a crappy diff tool.

Have you used SmartGit by Syntevo? I prepared all the changes checking with their diff tool, ensuring that you'd not have that problem. Grrrr.

Is there any chance you could try that one, or some other better quality diff tool? I actually changed very little of your code.

martinhbramwell commented 12 years ago

By the way ... I also forked and sent a Pull request for this :

https://github.com/davehunt/selenium-ide-flowcontrol

darrenderidder commented 12 years ago

That would be frustrating. Your issue might be character encoding or newlines on Windows. I believe the diff tool is accurately reporting that the line has been modified, if only in the whitespace or newline encoding. The solution is to use a text editor that preserves the original encoding for newlines and whitespace.

cheers, Darren

martinhbramwell commented 12 years ago

I tried again, working under Linux. Same deal.

I think the diff here freaks because the first lines are different.

I'll re-order it all.

martinhbramwell commented 12 years ago

You should be able to see the diffs now. I think the problem was gedit switching indent spaces to indent tabs. You might want to move the first comment block to the top where it belongs. I had also renamed the array "cycles" to "whileCmds" to make it's role a bit more obvious alongside my array "forEachCmds", but reverted that in order to keep the changes to additions only.

darrenderidder commented 12 years ago

Attempting to load sideflow.js fails: Failed to load user-extensions.js! files=/home/darren/Desktop/newsideflow.js lineNumber=259 error=SyntaxError: missing } after function body

It also looks like the main goto / while functionality has been commented out on line 99.

martinhbramwell commented 12 years ago

Please have a look here : https://github.com/davehunt/selenium-ide-flowcontrol/pulls

I will apply all further changes to that version as well.

Maybe at some point one of you will realize I'm sincerely trying to help with a valid contribution.

darrenderidder commented 12 years ago

Dave's is a repackage of this, so this would be the right place to submit contributions. However, code that 'breaks the build' simply can't be merged into the repo. At a minimum, make sure you verify that your code loads, passes all the regression tests, as well as any new test cases you've added. It looks like a useful addition, it just needs a little work to make it ready for merging.

martinhbramwell commented 12 years ago

I have, once again, complied with your requests.

You will find everything retested under FireFox 8 for both Ubuntu and Windows 7.

All changes have been applied to both forks: sideflow and selenium-ide-flowcontrol .

(The recent problem was to do with assuming (and hence not testing) that I was editing only comments. I recommitted without a last final test because I was utterly fed up with changing and rechanging my work for no other purpose than to find out how to satisfy a 3rd rate diff program that can't distinguish white space from code.)