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

Add UnixFileType so things work more smoothly on Windows. #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codypiersall
Copy link

@codypiersall codypiersall commented Aug 4, 2018

Instead of using argparse.FileType, this implements a subclass which ensures that newlines on Windows are '\n' instead of '\r\n'.

flamegraph.pl cannot parse the logs generated with carriage returns, so this PR prevents users on Windows from needing to do an extra step before running flamegraph.pl.

The implementation of the __call__ method was copy/pasted from Python's standard library.

I think a functionally equivalent change would be to define this function:

def file_for_writing(fname):
    if fname == '-':
        return stdout
    else:
        return open(fname, 'w', newline='\n')

and pass type=file_for_writing for the --output argument.

@codypiersall
Copy link
Author

Just wanted to ping you on this--what do you think about it?

@gunnihinn
Copy link

For what it's worth, I'm using this module on Windows, and when I run flamegraph.pl with Strawberry Perl, it's fine with the Windows line endings.

@codypiersall
Copy link
Author

@gunnihinn Gotchya. Thanks for the feedback. I can't remember what perl distribution I was using at the time, but I was probably using the one that ships with git (based on where perl, it's the first one in my path). Unix line endings should still work with Strawberry Perl though, I would think.

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