-
Notifications
You must be signed in to change notification settings - Fork 811
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
guile repl access to full api #1794
base: stable
Are you sure you want to change the base?
Conversation
There should be a Python option too if GnuCash is built with |
That's a cool idea and I had exactly the same remarks as @jralls |
63861f5
to
f510720
Compare
Session example
|
f510720
to
0aa8114
Compare
Here's an example fancy-add.txm which adds a split from the first to the second descendant account. It has some interactivity to input the Description. (removed, added as a commit) |
Very usable, and last comment amended to illustrate interactivity during cli session inputting the transaction description. |
Unsure about the |
07d0584
to
4bbc62b
Compare
Since it's command line you just want to start a python interpreter and apply the start-up code to |
loads the datafile Read-only, runs the script
will load the datafile R/W, runs the script (which may modify the datafile).
will load the datafile R/W, runs the script (which may modify the datafile), and launch the REPL.
should run python instead, when someone codes it. |
That all looks reasonable, except let's not use |
Ok. Will amend. Forgot one last option
will launch REPL with/without datafile without running script. |
You know that if the datafile doesn't have an option (canonically |
That's fine...I was simply hashing out the different possibilities for combining options and what's their intent.
|
6eb7ee7
to
bc0b4f6
Compare
be38331
to
20c8587
Compare
Before we dig into the details of option naming, I have a question: what is the advantage of passing a guile or python script to gnucash-cli over writing a guile or python script that loads the necessary gnucash (binding) modules and is then free to do whatever to a book (open it read only or read/write, extract data from it or add data to it) ? That's what the current python bindings offer already and ISTM it shouldn't be too hard to figure out how to do the same with guile bindings, though it may be missing some bootstrapping code. An interactive prompt via gnucash-cli could be handy though. And perhaps even that may be possible using the bindings directly instead of having to pass through gnucash-cli. Having an interactive prompt via gnucash-cli may be more convenient though so I'd keep that idea. |
That's it -- it lowers the barrier to entry tremendously, allowing experimentation from within a standard installation in linux/mac/windows. No need to hunt for bootstrap, no need to --include /weird/paths/to/library. No need to modify reports which crash to learn the API. No need to run a repl server and connect from telnet: https://wiki.gnucash.org/wiki/Custom_Reports#Command_line_access_to_the_API Users can save their scripts as simply |
Not really. The barrier isn't getting the REPL configured, the barrier is learning to program. This is a nice convenience for the few dozen users who can program in Scheme; adding Python support would be convenient for a few hundred more. That's still on the order of 1% or less of the users. |
So, what's the verdict on this branch? |
I don't have strong feelings either way. It adds convenience for a few people and seems mostly harmless otherwise. |
What is the meaning of REPL? |
I'll comment out the python parts then and merge in. I don't know how to provide parallel implementations in it. |
It's still theoretically possible with this branch to have a script open a new session, manipulate data, and close. |
gnucash/gnucash-commands.cpp
Outdated
#ifdef _WIN32 | ||
struct ConsoleStruct | ||
{ | ||
public: | ||
bool has_ansi () { return m_has_ansi; }; | ||
private: | ||
HANDLE m_stdoutHandle = INVALID_HANDLE_VALUE; | ||
DWORD m_outModeInit = 0; | ||
bool m_has_ansi = false; | ||
ConsoleStruct () : m_stdoutHandle {GetStdHandle(STD_OUTPUT_HANDLE)} | ||
{ | ||
if (m_stdoutHandle != INVALID_HANDLE_VALUE && GetConsoleMode(m_stdoutHandle, &m_outModeInit)) | ||
{ | ||
SetConsoleMode (m_stdoutHandle, m_outModeInit | ENABLE_VIRTUAL_TERMINAL_PROCESSING); | ||
m_has_ansi = true; | ||
} | ||
} | ||
~ConsoleStruct () | ||
{ | ||
if (m_stdoutHandle == INVALID_HANDLE_VALUE) | ||
return; | ||
printf ("\x1b[0m"); | ||
SetConsoleMode (m_stdoutHandle, m_outModeInit); | ||
} | ||
} ansi_codes; | ||
#else | ||
struct ConsoleStruct | ||
{ | ||
bool has_ansi () { return true; }; | ||
} ansi_codes; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A fancy method to set Win32 console to accept ansi colours. @jralls could offer testing please :) And the next step: a full-featured TUI for CRUD...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christopherlam I need an example script/input that will demonstrate the coloring.
And the next step: a full-featured TUI for CRUD...
We've already got one, it's called the register. Turning GnuCash into ledger-cli is an anti-goal. Frankly, so is expanding the reach of Guile in GnuCash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christopherlam I need an example script/input that will demonstrate the coloring.
And the next step: a full-featured TUI for CRUD...
We've already got one, it's called the register. Turning GnuCash into ledger-cli is an anti-goal. Frankly, so is expanding the reach of Guile in GnuCash.
Easiest way to test the ANSI coloring: launch GUI, open a test datafile, kill -9
to quit and leave datafile.LCK on. Then gnucash-cli datafile.gnucash --interactive --readwrite
to load the datafile and launch a guile session. The load will be interrupted by the Readonly/Unlock/Abort
prompt which should be in colour in all platforms including win32...
The CRUD would be a simple one, useful for a quick data entry; and an official hledger
output format would be nice instead of the https://hledger.org/gnucash.html. It would be possible to run such hacks like gnucash-cli datafile.gnucash --script book-to-ledger.scm | hledger -f- print
from https://hledger.org/1.31/hledger.html#standard-input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The register is a "simple" CRUD in that it allows the user to create transactions with splits and enforces the transaction balancing. The Transfer Dialog is another one. Neither is simple to code and there's a lot of functional duplication that makes for a maintenance headache. Adding a third implementation that interacts with the bindings--or worse, is written in Scheme or Python--is an incredibly bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I agree; the cli CRUD would follow from having the libgnucash/gnucash separation... as I understand they're separated because eventually it'd be nice to have a mobile app which can input data directly into the backend. A CLI data entry mechanism could be a good POC to illustrate best practice of using the engine to add data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? When we eventually achieve that GnuCash GUI will be turn into a suitable reference application. Note as well that neither Python nor Guile are available for programming on mobile. The choices there are Java and Swift, full stop. Admittedly at the rate we're going on the libgnucash separation that will probably have changed. After all, work on creating Swift started about the same time that we decided to make the separation.Regardless neither Python nor Guile are likely to turn up as a way to make mobile apps, ever.
b4ca4d1
to
d3c8cf8
Compare
gnucash/gnucash-core-app.cpp
Outdated
// don't process args on and after "--" | ||
std::vector<std::string_view> argv_vec (argv, argv + argc); | ||
auto match = std::find(argv_vec.begin(), argv_vec.end(), "--"); | ||
argc = std::distance (argv_vec.begin(), match); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to use standard algorithms (as I suggested in my previous review) but the bigger issue with this is that we (well, I...) chose boost::program_options for our command line handling and that this new code is working around it rather than using it to also handle the options to pass to scripts.
I am wary of using two code paths to parse options. Sooner or later they will start to interfere.
We may want to dig into more advanced use of boost::program_options to allow it to also be used to parse options to pass on to a script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boost::program_options
's positional options facility gets us partway there and is nicer than the std::find
hack. You're already using it for the input file and the std::find
hack might break that for your --Something.gnucash
example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok removed --
use and use --script-args abc def ghi
instead. The disadvantage is guile/python won't receive any args which begin with --
because boost will munge them.
chris@pop-os:~/sources/gnucash [env]$ ./stable-install/bin/gnucash-cli --interactive --script-args 1 2 3
This is a development version. It may or may not work.
Report bugs and other problems to [email protected]
You can also lookup and file bug reports at https://bugs.gnucash.org
To find the last stable version, please refer to https://www.gnucash.org/
Welcome to Gnucash Interactive Guile Session
(use-modules (gnucash core-utils))
(use-modules (gnucash engine))
(use-modules (gnucash app-utils))
(use-modules (gnucash report))
(use-modules (system repl repl))
(use-modules (ice-9 readline))
GNU Guile 3.0.9
Copyright (C) 1995-2023 Free Software Foundation, Inc.
Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.
Enter `,help' for help.
scheme@(guile-user)> (program-arguments )
$1 = ("gnucash-cli" "1" "2" "3")
scheme@(guile-user)>
chris@pop-os:~/sources/gnucash [env]$ ./stable-install/bin/gnucash-cli --interactive --script-args 1 2 3 -L python
This is a development version. It may or may not work.
Report bugs and other problems to [email protected]
You can also lookup and file bug reports at https://bugs.gnucash.org
To find the last stable version, please refer to https://www.gnucash.org/
Welcome to Gnucash Interactive Python Session
>>> import sys
>>> sys.argv
['1', '2', '3']
>>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disadvantage is guile/python won't receive any args which begin with -- because boost will munge them.
It doesn't munge them, it tries to parse them, fails, and puts up the usage message unless you either quote them as args to --script args
or by passing --
to read them as positional arguments.
bin/gnucash-cli --interactive --script-args "--1 --2 --3"
…
Enter `,help' for help.
scheme@(guile-user)> (program-arguments )
$1 = ("gnucash-cli" "--1 --2 --3")
scheme@(guile-user)>
Note that that while that protects the arguments from being parsed by boost::program_options it also turns them into a single option that needs to be separated.
bin/gnucash-cli --interactive -- --1 --2 --3
…
Enter `,help' for help.
scheme@(guile-user)> (program-arguments)
$1 = ("gnucash-cli" "--1" "--2" "--3")
scheme@(guile-user)>
But now the book file gets included in the script arguments instead of being passed to gnucash-cli to open a session:
bin/gnucash-cli myfile.gnucash --interactive --script-args "1 2 3"
…
Enter `,help' for help.
scheme@(guile-user)> (program-arguments)
$1 = ("gnucash-cli" "myfile.gnucash" "1 2 3")
scheme@(guile-user)>
And you're not giving the interpreter a shot at the options either:
bin/gnucash-cli --interactive -- '-c (program-arguments)' 1 3 3
…
Enter `,help' for help.
scheme@(guile-user)> (program-arguments)
$1 = ("gnucash-cli" "-c (program-arguments)" "1" "3" "3")
scheme@(guile-user)>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't munge them, it tries to parse them, fails, and puts up the usage message unless you either quote them as args to
--script args
or by passing--
to read them as positional arguments.
But now the book file gets included in the script arguments instead of being passed to gnucash-cli to open a session:
bin/gnucash-cli myfile.gnucash --interactive --script-args "1 2 3" … Enter `,help' for help. scheme@(guile-user)> (program-arguments) $1 = ("gnucash-cli" "myfile.gnucash" "1 2 3") scheme@(guile-user)>
I think this is fixed by limiting --input-file to 1 arg: m_pos_opt_desc.add("input-file", 1);
And you're not giving the interpreter a shot at the options either:
bin/gnucash-cli --interactive -- '-c (program-arguments)' 1 3 3 … Enter `,help' for help. scheme@(guile-user)> (program-arguments) $1 = ("gnucash-cli" "-c (program-arguments)" "1" "3" "3") scheme@(guile-user)>
I'm not sure this is possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok written to accept these 3 schemes. IMHO if the filename/uri is always pushed into argv[1] (even the empty filename
""
) then we can be very consistent.Do you mean as argv[1] to gnucash-cli ? That would make for a command line interface that's not very flexible and not really in the spirit of command line arguments.
No, I mean gnucash-cli's args are "rearranged" so that calling gnucash-cli --script script.scm --input-file datafile.gnucash --script-args 1 2 3
results in guile's (program-arguments)
returning '("gnucash-cli" "datafile.gnucash" "1" "2" "3")
.
Otherwise I like the idea of a top-level command with subcommands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ISTM argv[0]
should be either the interpreter name, i.e. guile
or python3
, or the script name. I'd lean toward the script name if it exists because it's a traditional unix practice to have hardlink aliases to a single program and the program does different things depending on the value of argv[0]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @gjanssens is on the right track with his modes suggestion. boost::program_options superficially appears to support it but bpo::command_line_parser
and bpo::store
are plain functions so I think we'll have to resort to using unknown_options to collect the secondary options and then reparse them with the mode's bpo::options_description
object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok perhaps this particular branch must be paused until the stable branch program_options are updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps. Rewiring the existing program options handling needs to be a separate commit but that doesn't rule out it being part of this PR.
e0b98c8
to
d95e5ae
Compare
IMV The most useful test will be from the guile repl with |
c63ae6d
to
48dc683
Compare
9aaf9e7
to
f057997
Compare
Here's an example of a runner calling a subprogram with arguments. https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-run In this case (while investigating https://bugs.gnucash.org/show_bug.cgi?id=799124) the CLI to use is
|
Which isn't gnucash-cli, so how is that germane? |
Simply to illustrate that using |
- and offer to [R]eadonly [U]nlock or [A]bort where appropriate.
Syntax: gnucash-cli [datafile.gnucash [--readwrite]] [--interactive] [--script script.py or script.scm] [--language python|guile] 1. --script or --interactive will launch scripting 2. a datafile will trigger loading, will attempt r/w if requested 3. --language python will choose python 4. --interactive will run the Guile or Python commandline with access to gnucash modules
gnucash-cli book.gnucash --script book-to-hledger.scm > ~/.hledger.journal hledger bs hledger bs Balance Sheet 2023-10-20 || 2023-10-20 =====================++============ Assets || ---------------------++------------ Assets:Current:Bank || -168.3 USD ---------------------++------------ || -168.3 USD =====================++============ Liabilities || ---------------------++------------ ---------------------++------------ || =====================++============ Net: || -168.3 USD
5e79d8a
to
de536c9
Compare
in case anyone wants to hack with guile repl
TODO
call-with-error-handling
to catch exceptions to clear session on exit or repl segfault: IMHO if an exception was raised, we should neatly close session without saving datafile, and disclose this on console.file_load
, instead of RW gracefully failing into RO mode, offer BREAK_LOCK as an option (but I'm slightly against unexpected interactivity)date, description, and formulated amount, and offer commit or rollback upon completion.