Add stdin support - addresses #53

[bryder]

Implements the filename - to indicate the input should be on stdin and the output on stdout.

I did some refactoring to make the code easier to understand when using stdin vs a normal file.

I separated the file I/O from the building of the diff or new file.

The types I use in the docstrings work with jetbrains tools.

To test it I added full integration tests for pyment rather than introduce mocking.

The tests work on windows and linux (tested using python2 and python3 on ubuntu, python2 on windows 10).

[dadadel]

Thank you @bryder for that work! That really cool :+1:

[alok]

@dadadel Could you put the new version of pyment up on PyPI so it doesn't require a pip install from source to get this feature?

[alok]

@bryder For outputting to STDOUT, I don't think a diff output is the best default. The STDIN reader should have a way to just output the source code directly, without overwriting or creating a diff.

[bryder]

Hi @alok

I tried to model the behaviour of pyment exactly. In -w mode (overwrite) you get the modified source on stdout, Without the -w flag you get the patch.

I couldn't decide if someone might want the patch instead of the new source on stdout so left it so that you could get either.

I also considered adding a couple of options --source-on-stdout --patch-on-stdout.

What do you think @dadadel / @alok ?

I'm happy to add those options. In that case if the input is a file or stdin I would put the source or patch on stdout and not write a new patch file or overwrite an existing file. The options would be mutually exclusive with -w

[alok]

I think that -w is a misleading name when you're outputting to STDOUT, since you aren't "overwriting" anything. I find tools like this to be best as UNIX filters, outputting modified source to STDOUT by default, with flags for patches and in-place modification. But I think the ship might have sailed for pyment being that sort of tool.

[alok]

Something like a --stdout flag is likely the best backwards-compatible option.

[dadadel]

Hey guys,

I think that it seems more natural to pipe on stdout a file content rather that a patch. But it's true that it would not follow the natural way of pyment workflow.

This could be a source of confusion. But I think it would be better to output by default the modified content instead of the patch by having by default. BTW, I don't know what would be the best.

[alok]

Since I'm generally in favor of doing the right thing, sometimes even at the cost of backward compatibility, I'd cut a v4 of pyment that makes output to STDOUT the default, and patches/overwriting flags.

Failing that, a --stdout flag is what I favor to avoid introducing special cases.

[bryder]

OK how about this?

• If the filename is - (on the command line) - input is stdin and output is modified source code on stdout.
• If no files are passed at all - input is stdin, output is modified source on stdout.

If the flag --patch-to-stdout is set - put the patch on stdout instead of the modified source.

That way the standard behaviours - - as a filename and no filenames - work as most would expect, and the default output is the new source not a patch.

It works as a filter with no extra options, and all of the previous behaviours still work.

[alok]

I'd actually drop the "no files passed" behavior. Pretty much all unix commands only support one or the other, not both. I think - as an explicit name is better, because otherwise scripts with commands like pyment "${arr[@]}" can fail if the array is empty, because pyment would hang waiting for STDIN that never arrives (I ran into this issue writing a script for yapf). The rest sounds fine to me.

[bryder]

I've seen plenty unix commands supporting both - hence the use of the '-' option. cat(1) for example.

It's important the script can continue to take a directory name for bulk operations, and an individual file name because if you tried to do this

cat /some/file | pyment > /some/file

You will end up with an empty file.

The technique I use to handle people not realising the command is wating on stdin is to print a warning on /dev/tty (or stderr) if the input is a tty - not a file (or to whatever the windows equivalent is to /dev/tty).

It's a simple way to handle that case.

On Tue, 19 Jun 2018 at 09:39, Alok Singh notifications@github.com wrote:

I'd actually drop the "no files passed" behavior. Pretty much all unix commands only support one or the other, not both. I think - as an explicit name is better, because otherwise scripts like pyment "${arr[@]}" can fail if the array is empty, because pyment would hang waiting for STDIN that never arrives (I ran into this issue writing a script for yapf). The rest sounds fine to me.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dadadel/pyment/pull/59#issuecomment-398204380, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWEvLQMJZ2GrJkSfOVylOLiy9ILOAKmks5t-B4GgaJpZM4UqnLy .

[alok]

Eh, I don't have a strong opinion on this. Your suggestion above my last comment sounds fine.