Fixes #1994 #4874

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@flyx
Contributor
flyx commented Oct 9, 2016 edited

This PR adds gorgeEx and staticReadEx which both return a tuple of output and result code. Furthermore, it add an optional parameter onErrorBreak to gorge and staticRead, because that was cheap to add when implementing gorgeEx functionality.

staticReadEx and gorgeEx will still break if executing the command raises an OSError.

This PR includes #4872.

@yglukhov
Member

So in what way #4871 is fixed? Any chance to note that in news.txt?

@flyx
Contributor
flyx commented Oct 25, 2016

@yglukhov sorry, just saw that comment. The fix for #4871 has already been merged by #4872, so this seems to be the wrong place to discuss it. @Araq already documented it in the news.

@Araq
Member
Araq commented Oct 25, 2016

I think this implementation is convoluted. Internally opcGorge can be changed to opcGorgeEx and a wrapper gorge can call gorgeEx. I'm also not too happy with this Ex naming convention.

@flyx
Contributor
flyx commented Oct 25, 2016

@Araq: I had problems determining which proc was called without splitting opcGorge and opcGorgeEx. Not sure what and how you want it to be changed.

What would you prefer as name for the extended procs?

flyx added some commits Nov 14, 2016
@flyx flyx Tried to make code less convoluted
 * moved common opcGorge[Ex] code to template
 * renamed opGorge to opGorgeEx
 * splitted opcGorge / opcGorgeEx implementations
31d5b07
@flyx flyx Added gorgeEx test 38f4fe5
@flyx flyx Merge remote-tracking branch 'upstream/devel' into staticreadex 8586ef3
@Araq
Member
Araq commented Jan 7, 2017

Implemented it differently. Read and learn.

@Araq Araq closed this Jan 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment