mondeja / mdpo

Markdown files translation using GNU PO files
https://mondeja.github.io/mdpo/
BSD 3-Clause "New" or "Revised" License
25 stars 6 forks source link

Positional `GLOB_OR_CONTENT` argument should supersede STDIN. #159

Closed EuAndreh closed 3 years ago

EuAndreh commented 3 years ago

I've encountered this issue when running a Git post-receive hook an a server.

On such hook, Git does a little preparation, setting some environment variables and also providing some values via STDIN. From https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks:

It takes a list of references that are being pushed from stdin (...)

The main ones are oldrev, newrev, ref, and they're commonly accessed this way:

read oldrev newrev ref

# Do something with those variables

The issue I encountered was: when running mdpo locally, everything would work, but when on the Git hook on the server, mdpo would emit empty files. It wasn't immediatelly obvious because the call to mdpo was deep in a series of sh script calls, but I got a minimal reproducible example working.

Sample interactive session:

$ pushd `mktemp -d`
/tmp/tmp.nJ3utb81GJ ~
$ echo 'the content' > sample.md
$ md2po --quiet --po-filepath l-interactive.po --save sample.md
$ po2md --quiet --pofiles l-interactive.po --save sample.l-interactive.md sample.md
$ cat sample.l-interactive.md
the content
$ echo | md2po --quiet --po-filepath l-piped.po --save sample.md
$ echo | po2md --quiet --pofiles l-piped.po --save sample.l-piped.md sample.md
$ cat sample.l-piped.md
$ echo | md2po --quiet --po-filepath l-piped-stdin.po --save < sample.md
$ echo | po2md --quiet --pofiles l-piped-stdin.po --save sample.l-piped-stdin.md < sample.md
$ cat sample.l-piped-stdin.md
the content
$
$ md2po --version
.md2po-real 0.3.54

Extracted steps to reproduce:

pushd `mktemp -d`
echo 'the content' > sample.md

md2po --quiet --po-filepath l-interactive.po --save sample.md
po2md --quiet --pofiles l-interactive.po --save sample.l-interactive.md sample.md
cat sample.l-interactive.md

echo | md2po --quiet --po-filepath l-piped.po --save sample.md
echo | po2md --quiet --pofiles l-piped.po --save sample.l-piped.md sample.md
cat sample.l-piped.md

echo | md2po --quiet --po-filepath l-piped-stdin.po --save < sample.md
echo | po2md --quiet --pofiles l-piped-stdin.po --save sample.l-piped-stdin.md < sample.md
cat sample.l-piped-stdin.md

md2po --version

I believe the cause is that the sys.stdin.isatty() check on __main__ is getting more precedence than the positional interpretation of GLOB_OR_CONTENT.

The behaviour I expected to encounter is the opposite, and when given a positional argument, it supersedes the check of being invoked directly or inside a pipe. Take cat, for example:

$ echo 'the content' > f.txt
$ cat f.txt
the content
$ echo | cat f.txt
the content
$ echo | cat -

$ echo | cat

I didn't investigate whether this is an issue with the argparse library or with its usage in __main__, though, but I think the problem is restricted to the code region.

As you can see above, the way I found around this issue was to always feed the input to mdpo via STDIN, using md2po < f instead of md2po f, but I believe that this is not the intended behaviour.

WDYT?

mondeja commented 3 years ago

That logic was rewritten in v0.3.57, so could you check if it is still failing in the latest mdpo version (v0.3.63)?

EuAndreh commented 3 years ago

I'll update to v0.3.63 and check it.

EuAndreh commented 3 years ago

It indeed worked correclty. Thanks!