Changed env var check to a more elegant approach. Thanks, @bitprophet! #1271

Merged
merged 3 commits into from Apr 7, 2016

Projects

None yet

3 participants

@bergbrains
Contributor

Jeff,

This is cleaned up and changed as per recommendations.

@bitprophet bitprophet added the Feature label Feb 28, 2015
@bitprophet bitprophet added this to the 1.11 milestone Feb 28, 2015
@kaaveland kaaveland commented on an outdated diff Jan 25, 2016
fabric/colors.py
def _wrap_with(code):
def inner(text, bold=False):
c = code
+
+ if os.environ.get("FABRIC_DISABLE_COLORS", None):
@kaaveland
kaaveland Jan 25, 2016

This should probably check if is None, if the intention truly is to have any value for FABRIC_DISABLE_COLORS work. It's possible to set it to the empty string and this would then disable the colors. So either the documentation or the code should be updated to stay in sync.

Eric D. Berg added some commits Feb 5, 2016
@bitprophet bitprophet merged commit 7cfdc3c into fabric:master Apr 7, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@bitprophet bitprophet added a commit that referenced this pull request Apr 7, 2016
@bitprophet bitprophet Changelog re #1271 ed29d44
@kaaveland kaaveland commented on the diff Apr 8, 2016
fabric/colors.py
def _wrap_with(code):
def inner(text, bold=False):
c = code
+
+ if os.environ.get('FABRIC_DISABLE_COLORS') is None:
@kaaveland
kaaveland Apr 8, 2016

I think maybe this contradicts the doc above now -- we'll only exit early here if FABRIC_DISABLE_COLORS is not set?

@bitprophet
bitprophet Apr 9, 2016 Member

Jeez, yea. Thanks! I didn't even test this by hand :( Just did now to confirm. Pushed fix. Wish the history here was clearer so I could remember what the original approach was, but meh. Works as described now. 4ed5c94

@bergbrains
bergbrains May 11, 2016 Contributor

Great. Thanks, Jeff.

I'm an old Perl hacker and was hoping to get a better take on how to
manipulate the fab run environment with command line or config file options
too. haven't quite figured that out, but an env var is good for batch
processes.

Thanks again.

Eric

On Fri, Apr 8, 2016 at 11:35 PM, Jeff Forcier notifications@github.com
wrote:

In fabric/colors.py
#1271 (comment):

def _wrap_with(code):

 def inner(text, bold=False):
     c = code
  •    if os.environ.get('FABRIC_DISABLE_COLORS') is None:
    

Jeez, yea. Thanks! I didn't even test this by hand :( Just did now to
confirm. Pushed fix.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/fabric/fabric/pull/1271/files/7cfdc3ce430add37a3206aa959b7ec55cd0e813e#r59109647

eric berg
eberg@bergbrains.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment