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

Workaround for broken format on Windows #9

Merged
merged 4 commits into from
Sep 12, 2016

Conversation

daniel-liuzzi
Copy link
Contributor

@daniel-liuzzi daniel-liuzzi commented Aug 27, 2016

Before

1-before

After

2-after

Essentially, this trades yellow refnames with proper alignment. Once column is fixed, this can then be burned to ashes. This only affects Windows.

Ref. #8

@paulirish
Copy link
Owner

I think we can do this a little more elegantly. Like building our format string using a few bash variables, and we'll only branch for the first. Know what i mean?

@daniel-liuzzi
Copy link
Contributor Author

If you mean to remove all duplication, I totally agree. I was going to do just that. If you that's not what you mean, then forgive my ignorance. This is the first time ever I work on a bash script!

The reason I did it like this is because I don't have access to a proper console (i.e. one where column works), so I can't test that use case. Also since this is a (hopefully) temporary fix, I did it all ad-hoc, so removing it when it's no longer needed is as simple as deleting it.

I'll try and set up a Linux VM to give this another shot.

@daniel-liuzzi
Copy link
Contributor Author

daniel-liuzzi commented Aug 31, 2016

Tried this on Git Bash on Windows as well as on "Bash on Ubuntu on Windows" (that's a mouthful). Both seem to work properly. Please try it on macOS when you get a chance, as I don't have access to a Mac computer. I have also reported the column bug (git-for-windows/git#865).

@paulirish
Copy link
Owner

breaks on mac:

image

- Make OS detection logic more POSIX-ish
- Add CYGWIN and CYGWIN Windows variants
@daniel-liuzzi
Copy link
Contributor Author

daniel-liuzzi commented Sep 3, 2016

Fixed. Tested on Mac, Linux, and Windows. Working consistently across all. Sorry about that.

@paulirish
Copy link
Owner

looks good. thx!

@paulirish paulirish merged commit d45fb14 into paulirish:master Sep 12, 2016
@daniel-liuzzi daniel-liuzzi deleted the 8-win-workaround branch September 17, 2016 20:31
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