add support for command line arguments to be passed on to python script (-py option)

[berteh]

adding 2 options to pass arguments to python script run in Scribus from command line (to be used with -py and -g), after short discussion with a-l-e on irc.

--python-arg<argumentname> <value>   Argument passed on to python script with its value, no effect without -py
--python-flag<flagname>              Argument passed on to python script with no value, no effect without -py

[luzpaz]

@berteh Check out: https://travis-ci.org/scribusproject/scribus/jobs/74267458#L4384

[aoloe]

all in all: looks like a good job.
i would love it to see the two improvements i'm suggesting inline (adding a dash and args only), but it's not up to me to decide :-)

[luzpaz]

I'm gonna try to build on OSX. will report back

[berteh]

Hello.
@aoloe The reason I did not use --python-arg- as a prefix is to allow in a cleaner way to pass both short and long arguments to the script, ie: --python-arg-o <file> would pass -o <file> and --python-arg--outfile <file> --python-arg--infile <file> would pass --outfile <file> --infile <file>.
Having one additional dash for long arguments seemed awkward as they already have 2. Maybe it'd be better to make it more explicit in the documentation, ie call it "argument" instead of "argument name" in the usage info?
But if you like it more yes, I can easily have the dash in the prefix, and add-it automatically before passing to the script to achieve the same effect.

@aoloe The reason I had to separate args and flags is simply because I did not want to presume too much about the syntax of the script arguments... and to handle the final (scribus) positional argument (filename) correctly: otherwise how do you know if scribus -py myscript.py --python-arg-o myfile.sla should pass the last filename to the script or open it in scribus?

Another option I see feasible is to have a single argument, that simply superseeds the standard command line arguments parsing mechanism and pass everything that follows to the script, not event parsing it.... seems quite clean to me, but maybe less conform to classical use, i.e. scribus -py myscript.py --python-args -d --prefs file1 --out file2 myfile.sla would then pass to myscript.py all of -d --prefs file1 --out file2 myfile.sla

@luzpaz thanks for the test.

[berteh]

@luzpaz https://travis-ci.org/scribusproject/scribus/jobs/74267458#L4384 is a warning in objprinter.cpp ... I changed no code in there. want me to have a look to try fix the warning?

[luzpaz]

@berteh Thanks! But, better to keep the patch very concise. Lets not fix issue unrelated to what you are trying to achieve here. Better to submit a separate patch for a different fix. It's less auditing work for the devs.

[luzpaz]

@berteh another thing. Can you give some instruction on how to test this ?

[luzpaz]

FYI, it's cool that you can throw .diff or .patch at the end of a github url like https://github.com/scribusproject/scribus/pull/19.patch and it will generate a legit .patch. That's how I tested this patch via OSX: https://gist.github.com/luzpaz/69c00f2a8516b5462614#file-scribuspatch-rb-L41-L44

[berteh]

@luzpaz easiest way to test is, to me, to run a simple python script that prints any argument it gets (see below) and call if from command line via scribus

save the script below as ~/cmdtest.py

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import sys

import sys, scribus, traceback

def main(argv):
    print "\nYES! main is reached in cmdtest script\ncheck out the arguments it's got:", " ".join(argv)
    if scribus.haveDoc():
        print "\nthis script could modify the current document ", scribus.getDocName(), " automatically"
    else:
        print "\nno document is currently open in scribus"

def main_wrapper(argv):
    try:
        if scribus.haveDoc():
            scribus.setRedraw(False)
            #scribus.statusMessage("ĥeadless test")
            #scribus.progressReset()
        main(argv)
    except Exception:
        print "error: "+traceback.format_exc()
    finally:
        # Exit neatly even if the script terminated with an exception,
        # so we leave the progress bar and status bar blank and make sure
        # drawing is enabled.
        if scribus.haveDoc():        
            scribus.setRedraw(True)
        scribus.statusMessage('')
        scribus.progressReset()

if __name__ == '__main__':
    main_wrapper(sys.argv)

running scribus with this patch via

scribus -g -py ~/cmdtest.py --python-arg-arg1 value1  --python-arg--flag1 --python-arg-v value2 

would then output

YES! main is reached in cmdtest script
check out the arguments it's got: cmdtest.py ext -arg1 value1 --flag1 -v value2

no document is currently open in scribus

and running it with an additional sla file (that needs to exist on your machine):

 scribus -g -py ~/cmdtest.py --python-arg-arg1 value1  --python-arg--flag1 --python-arg-v value2  --python-arg-flag2 -- ~/some/existing/scribus/file.sla

would generate the following output... proving the script has been running properly.

YES! main is reached in cmdtest script
check out the arguments it's got: cmdtest.py ext --arg1 value1 --flag1 -v value2 -flag2

this script could modify the current document  /home/<user>/some/existing/scribus/file.sla  automatically

[luzpaz]

@berteh Excellent. The patch works!!
Some observations:

1. when specifying an .sla the script will execute and then exit scribus. Is that what we want ?
2. As a refinement perhaps we can forgo showing the splash screen when telling scribus to exectute python scripts from the CLI.

[aoloe]

@berteh

tldr; why should somebody care if the parameters are passed with one or two dashes? why can't we stick to the one dash fits all?
isn't --python-arg-i more readable than --python-argi ?

and here are the details... :-)

• --python-argi is imo less readable than --python-arg-i...
• i think that we should only support short arguments:
  • --pyhton-arg-v --python-arg-o <file> --python-arg-infile <file> should imo pass -v -o <file> -infile <file> to the python script
  • or only long arguments (why not? )
  • or automatically use short arguments for one letter arguments and long arguments for longer ones.
    i don't really care...(even if i prefer the first solution with the short arguments)
    but i'm not fond of leaving the decision to the user... (except if there are good reasons for it... i don't see any, but you might...)
• i don't 100% like -py setting the name of the script to be run... but, if we stick to that, the args should probably be passed as --py-arg-v.
  my preferred solution would probably be to add --python-run <file>, deprecate --py <file > and remove the second way in the new scripter. and stick to --python-arg- (all that done in a further patch).
• scribus -python-run myscript.py --python-arg-o myfile.sla: good catch!
  i think that myfile.sla should be passed to -o. i see two ways of handling this correctly:
  • once --python-run is set, the filename is ignored by scribus. always (imo, it should be the script that opens the file, not scribus... but i agree that sometimes it's nicer to see a filename at the end of the command...);
  • add a void -- argument to tell that the arguments are finished: scribus -python-run myscript.py --python-arg-o -- myfile.sla would get scribus to open my file and launch myscript.py with the -oargument.

personally, i prefer a clean, non-ambiguous and simple syntax (even with longer argument names, since you're not really supposed to type all of that, but rather put them in a script...) to complex ways to accommodate the wishes of every user...

something like:

--python-run <file-name.py> Python file to be run on startup
--python-arg-<argument-name>[ <value>]   Argument passed to the python script, optionally with its value; mutliple arguments are accepted; the arguments are ignored if --python-run is missing.
-- end of arguments

a long comment, ... and not much different than my previous one... sorry...

but i think it's important to get the interface right... history has proven that later changes are hard to get into the trunk :-(

[berteh]

@luzpaz < "when specifying an .sla the script will execute and then exit scribus. Is that what we want ?"

yes if you specify -g option (as i guess you did). If not, scribus opens the sla, runs the script and does not exit.

@luzpaz < forgo showing the splash screen when telling scribus to exectute python scripts from the CLI.

+1, but I'd rather bind the no-splash to the "-g" (no-gui) option, and not to the script thing... but you can as well simply add -ns option to get rid of it.

@aoloe < --python-argi is imo less readable than --python-arg-i...

agreed, and --python-arg-i IS the way it is handled in the current patch. I'll just adjust the code so it shows more.

@aoloe < only long arguments (why not? ) ; why should somebody care if the parameters are passed with one or two dashes

1 or 2 dash is a mere convention. Usually in linux you have 1 dash for short argument form (typically single letter, like -d) and 2 dashes for synonym long form (like --debug)... it's standard in most Python arguments parsing librairies, like argparse and both are supported by the current patch, to the preference of the script author, for both arguments and flags, without any extra code... so I'd keep it that way, no need to enforce a single dash.

@aoloe < once --python-run is set, the filename is ignored by scribus and must be opened by the script.

would look much nicer from the command line, but requires more work in refactoring all scripts to work "from command line", as most currently are modifying the document already opened in scribus... which is directly supported if you "let" scribus open the last argument.

@aoloe < i prefer a clean, non-ambiguous and simple syntax (even with longer argument names)

agreed. then I'd not ignore the last (sla file) optional argument, as it would be ambiguous.

about clean, how is the following not clean, to you? (so I know what to try to improve)
scribus -g -ns --python-script ~/cmdtest.py --python-arg-o value1 --python-flag-f --python-arg--long-arg value2 --prefs ~/specific/project/settings.xml ~/some/existing/scribus/file.sla

what I like is you can directly see what goes to scribus and what goes to the script independently of their order on the command line.
what I don't like is it's a bit too verbose, but that's okay

[aoloe]

@berteh

to me --python-arg--long-arg is not clean... --python-arg-long-arg is unambiguouser :-)

and i really would prefer:

scribus -g -ns  --prefs ~/specific/project/settings.xml --python-script
~/cmdtest.py --python-arg-o value1  --python-arg-short-arg value2 --python-arg-f --
~/some/existing/scribus/file.sla

i don't quite see why we should have -- inside of the argument name or have two arguments for flags and args... except if there is a need to keep them apart in python.

scribus has a long history of "we do it because we can (and because somebody might want it)" and this makes the life harder for the users and for the programmers :-(
i'm more and more keen to stick to what is really needed.
(and passing arguments is really needed! :-)

add the features you can't keep out of the code base :-)
a.l.e

[berteh]

@aoloe ok. if you prefer -- I can work from there even if I like it less.... but then there's no need for so much typing, a single argument would be enough to pass even multiple ones:

scribus -g -ns --prefs ~/specific/project/settings.xml --python-script ~/cmdtest.py --python-args -o value 1 -short-arg value2 -f --long-flag -- ~/some/existing/scribus/file.sla

the new --python-args would simply pass anything that follows to the script, until --(or any other "stop" token) or the end of command line, whichever comes first... in this case -o value 1 -short-arg value2 -f --long-flag

Would that be better to you?

[aoloe]

the -- was meant as a general command to stop checking for arguments...

not specific for the scripter...

i've already seen that in other tools, that's why i thought about it.

(and no, i would not prefert to have all python args in one block :-)

[berteh]

Ok. I'll try to do that... but it seems more difficult to explain (to me) than the flag/arg mechanism.

[berteh]

in fact... I cannot come up with a simple sentence to explain it in the usage info. How would you explain it please?

tried the following but it turns awful:
ts << (QString(" %1<%2> [%3] [--] ").arg(ARG_SCRIPTARG_PREFIX).arg(tr("argumentname")).arg(tr("value"))) << tr("Argument passed on to python script, with an optional value, no effect without -py. Add '--' after your last argument to be passed to the script."); endl(ts);

[aoloe]

i would say:

ts << (QString(" %1<%2> [%3] [--] ").arg(ARG_SCRIPTARG_PREFIX).arg(tr("argumentname"))
.arg(tr("value"))) << tr("Argument passed on to python script, with an optional value"); endl(ts);
ts << (QString(" %1").arg('--')) << tr("end of arguments list."); endl(ts);

(not tested)

the -- is not directly related to the scripter.

[berteh]

... hum that does not explain why you'd still be allowed to put the positional (file) argument behind

[aoloe]

i googled for it (and it was not easy to google for -- :-), and it seems to be slightly different than what i was thinking:

http://unix.stackexchange.com/questions/11376/what-does-double-dash-mean-also-known-as-bare-double-dash

but it's not that far...

i think that we should just document what the meaning of -- is. the man can then explain when it's useful...

[berteh]

ok. thanks. learned something today ;)

[luzpaz]

FYI Once we iron out all the details we need to submit patch to bugs.scribus.net (this is how the devs prefer). Github master is meant to only act as a mirror.

[berteh]

@aoloe implemented per your request as only --python-arg-<argName> [optionalValue], with double-dash support to end options sections. Jointly, this works in all cases i could think of, so I'm happy ;)

@luzpaz thanks for the info, I'll wait for your feedback before doing that.

[aoloe]

@berteh tomorrow i'll have some time and have a look at the code!

thansk for your work (and your patience!)

[berteh]

@luzpaz I was just procrastinating on this. now it's done. sorry for the delay.

[berteh]

submitted at http://bugs.scribus.net/view.php?id=13311

[luzpaz]

@berteh 👍

[aoloe]

nice! thanks @berteh for this patch!

[berteh]

syntax of command line updated per Craig request:

 -pa,  --python-arg <name> [value]      Argument passed on to python script, with an optional value, no effect without -py
 --                                     Explicit end of command line options

[luzpaz]

@chhitz this PR was submitted to SVN and was accepted. We're trying to figure out a way to give the correct credit to folks submitting PRs that get committed via SVN instead of just closing the PR without attribution or success. One way is to commit and then revert the commit and then wait for the changes from SVN to get pushed to github. Can we add this function to svn-to-git script that you made ?

CC @aoloe

[luzpaz]

Sorry. pushed wrong button to comment.

[berteh]

would be nice indeed. thanks for suggesting it.

[luzpaz]

Looks like there is a regression...?
http://bugs.scribus.net/view.php?id=13380

[berteh]

was one indeed. fixed by Craig in 011680a, thanks!

[luzpaz]

Craig closed ticket on BT http://bugs.scribus.net/view.php?id=13311
So how would be close this PR and still credit you with a successful PR. @aoloe said we could use
the Github 'command line instructions'

[aoloe]

eh eh, no i said that we should check them and see if they allow us to accept a PR without touching the commits list...
but i would not try it, before having discussed it with chitz!

[luzpaz]

@berteh check out http://bugs.scribus.net/view.php?id=13408
CC @aoloe

[berteh]

@luzpaz got your ping, thanks!

[luzpaz]

@berteh
Craig committed http://bugs.scribus.net/view.php?id=13415
does this interfere with anything ?