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

Fixes bug #6838, quoteValueList fails with multiple empty values. #6839

Closed
wants to merge 3 commits into from
Closed

Fixes bug #6838, quoteValueList fails with multiple empty values. #6839

wants to merge 3 commits into from

Conversation

OakBehringer
Copy link

Link to bug:

#6838

This may exist in other db platforms if the same do loop syntax is used. It looks like my whitespace (tab character) is different than yours? Anyway, the only code actually changed is that residing in the function quoteValueList, starting on line 157.

Cheers.

@Martin-P
Copy link
Contributor

Martin-P commented Nov 3, 2014

It looks like my whitespace (tab character) is different than yours?

Please use ZF2 coding standard which is 4 spaces for indent, not 6.

@OakBehringer
Copy link
Author

@Martin-P size is set to 4 chars in my IDE (PhpStorm).

http://snag.gy/ECtAw.jpg

Is the tab character acceptable for the ZF2 coding standard?

@Martin-P
Copy link
Contributor

Martin-P commented Nov 4, 2014

Unfortunately you need to use 4 spaces for indent. It would become somewhat messy if everyone uses his own "coding standard".

Looking at your image, you need to uncheck Use tab character. See also: http://www.jetbrains.com/phpstorm/webhelp/code-style-php.html#d310871e411

When the check box is cleared, PhpStorm uses spaces instead of tabs.

@OakBehringer
Copy link
Author

Martin,

That's exactly what I was asking-is the tab character acceptable. I personally prefer it over 4 spaces, but I am happy to switch over to spaces when contributing to ZF2.

I have updated my fork/branch so that the file uses the zf2 coding standard.

Cheers,
Adam

@turrsis
Copy link
Contributor

turrsis commented Nov 19, 2014

This is fixed here 01b9000

@Ocramius
Copy link
Member

As @turrsis explained, this was fixed on merge in #5683, though you may still add a test case to prevent a regression (I didn't test this particular bug, I just removed weird looping logic)

@Ocramius Ocramius closed this Nov 19, 2014
@Ocramius Ocramius added this to the 2.4.0 milestone Nov 19, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants