Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colorize git commands run through gitdist #205

Merged
merged 2 commits into from
Jul 17, 2017

Conversation

jmgate
Copy link
Collaborator

@jmgate jmgate commented Jul 12, 2017

It was bugging me that gitdist stripped out whatever colors would normally be displayed when running the equivalent git commands from the command line. This is an attempt to fix that problem by tying the stdout and stderr of the subprocess used to execute the git command to the stdout and stderr of the process running gitdist itself. Not sure if that's the best way to do this—I only have about an hour's worth of experience with Python's subprocess. I haven't seen any unintended side effects of this change yet, though I am by no means a power gitdist user. Let me know if this just isn't a good idea, or if we need to go about it a different way. Many thanks.

When opening a subprocess and passing the stdout to PIPE, the output is
buffered, losing the colors otherwise seen when running the command from
the command line.  One solution to keeping the colors is to redirect the
stdout and stderr of the subprocess to the stdout and stderr of the main
process.  Calling communicate() then makes the call block until the end
of the subprocess, otherwise gitdist would continue while the previous
git command was finishing, leading to interspersed lines of output.
@bartlettroscoe
Copy link
Member

This is so odd, but I never got an email from GitHub about the creation of this issue! Is GitHub broken or something? Is the SNL system filtering these emails as spam? What is going on?

@jmgate
Copy link
Collaborator Author

jmgate commented Jul 17, 2017

Is GitHub broken or something? Is the SNL system filtering these emails as spam?

@bartlettroscoe, I'm not sure. I was messing with my notification settings last week to try to decrease the number of emails I get, and I think at this point I don't get any notifications from Trilinos or TriBITS, so I'm probably not the best person to ask. Except I just got an email with your last comment, so now I'm rather confused.

With the change in the runCmnd() function, you need to add a print("")
statement to keep two blank lines after the end of the command and the next
git repo header output.  I think having two newlines is a good separator
between git repo output.
@bartlettroscoe
Copy link
Member

The commit 0518ce8 I just pushed fixes the gitdist test suite with Python 2.6.6.

I will go ahead and merge and push this to the 'master' branch.

Thanks for fixing this! It has annoyed me for a long time that running gitdist removed color (but color still gets removed when I pipe it into less for some reason).

@bartlettroscoe bartlettroscoe merged commit 0518ce8 into TriBITSPub:master Jul 17, 2017
bartlettroscoe added a commit that referenced this pull request Jul 17, 2017
Addresses #205

Build/Test Cases Summary
Enabled Packages:
Enabled all Packages
0) MPI_DEBUG => passed: passed=246,notpassed=0 (0.62 min)
1) SERIAL_RELEASE => passed: passed=246,notpassed=0 (0.61 min)
2) MPI_DEBUG_CMAKE-3.6.2 => passed: passed=265,notpassed=0 (0.66 min)
3) SERIAL_RELEASE_CMAKE-3.6.2 => passed: passed=265,notpassed=0 (0.50 min)
Other local commits for this build/test group: 0518ce8, fcb7ac2
@jmgate
Copy link
Collaborator Author

jmgate commented Jul 17, 2017

(but color still gets removed when I pipe it into less for some reason)

That's because command output gets buffered as it's sent through the pipe. Command line fixes for that include prepending your command with unbuffer from the expect package or stdbuf -o0 -e0 from, I think, GNU coreutils. Unfortunately those fixes aren't necessarily portable.

@jmgate jmgate deleted the gitColorsWithGitdist branch July 17, 2017 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants