OpenBB
OpenBB copied to clipboard
[Bug] slash issue
Describe the bug A "bad" load from the econometrics menu will crash the terminal. "Bad" loads have included so far: not giving an alias along with the filename, and setting the filename directly without using the auto-complete feature (which is dope, ty.)
To Reproduce
from the terminal, enter the econometrics menu and try to explicitly load a filepath (i used one i just stored from FRED data) - load -f /home/<user>/code/OpenBBTerminal/exports/icsa-test.csv
with or without an alias
it fails, load: error: argument -f/--file: expected at least one argument
which it not accurate, as it was given an argument.
Screenshots
Desktop (please complete the following information):
- OS: (Host) Mac OS X 12.2.1 running Parallels running (Guest) Ubuntu 20.04.2 ARM64
- Python version = 3.8.13
Additional context Should be easy to reproduce, hmu in Discord otherwise, happy to help troubleshoot.
@andr0id3000 Looks like an issue here: https://github.com/OpenBB-finance/OpenBBTerminal/blob/5cc2ee9e142837b5d92750c5bd43a82d1419e1c8/openbb_terminal/parent_classes.py#L160-L171
If it finds the /
character anywhere in your input it splits your command into seperate commands by /
(if I explained it properly)
For example, your command (load -f /home/q/code/OpenBBTerminal/exports/icsa-test.csv
) gets changed to:
$ load -f
gives the error
$ home
changes to home
$ q
quit the program
I don't know why it does this, but we should probably change the character used to something like ;
so we can enter paths.
You could go into the afformentioned piece of code and change: https://github.com/OpenBB-finance/OpenBBTerminal/blob/5cc2ee9e142837b5d92750c5bd43a82d1419e1c8/openbb_terminal/parent_classes.py#L161
to
elif ";" in an_input:
and https://github.com/OpenBB-finance/OpenBBTerminal/blob/5cc2ee9e142837b5d92750c5bd43a82d1419e1c8/openbb_terminal/parent_classes.py#L162
to
actions = an_input.split(";")
I'll open a PR if this fixes it for you.
For right now the correct way to do this would be to send arguments with a /
character in them through quotation marks like this: load -f :/home/<user>/code/OpenBBTerminal/exports/icsa-test.csv"
. However, I understand this is unintuitive and will get with the team on the best resolution of this. Thanks for the great explanation @bolshoytoster.
This is exactly right. This is something related with #1599 and it's down to the way we allow the user to create a queue of commands.
This is also how we are able to run routines. We rely strongly on the "slash" separator, thus when a command requires it, this issue occurs.
Examples are exe
or star history
on the new open source menu. The way we have been doing it is consuming the next command from the queue and updating the queue, which is very hacky and definitely a temporary solution.
I will have a go at a solution for this tonight. I will try to basically create a flag where in certain commands the user will not be able to use the slash operator. This is the only think I can think since it's impossible to allow for both as you wouldn't know when user wanted to use slash for another command or for a path.
@DidierRLopes you could instead use a different character, I suggested ;
because it's commonly used in things like bash already.
Is there a way to check if the command is an argument if the path does not exist, or to check throw up a warning that says "if there is a /
in the comments use quotations. Also, could we look into having maybe using \
and /
differently to create a fix (or would this add more confusion).
@DidierRLopes you could instead use a different character, I suggested
;
because it's commonly used in things like bash already.
I think this is the better idea. Has its downsides of course, but will quickly resolve the issue.
@bolshoytoster this would work, the issue I see here is that it is unnatural for users to do that when specifying a path. Even worst because they can copy-paste quickly a path from their OS and paste it on the platform, in which case that would fail.
@colin99d I think if we were to follow this approach, we would be better to just accept "" instead. because that way at least the Windows users could still copy-paste paths. Thoughts?
@DidierRLopes sorry, I meant using ;
for separating commands - not paths.
I thought we would keep the file navigation paths normal, and then make navigating through the terminal different. This is a big change tho. A smart way to tell the user to add quotation marks might be better. Maybe we could mark those commands in a certain way so that they are aware
Wait, since commands should never have navigation after them there has to a way to just create logic for this
Ohhh @bolshoytoster I see!! I like that!! It's not as visible as "/" but I think is less confusing in the long run.
@colin99d ?
I was originally thinking the same thing as @bolshoytoster, but do you think we can just build logic into the parser. Because commands will never have navigation after them.
@colin99d what do you mean by navigation after them?
@bolshoytoster we love the idea! Were you wanting to do the PR for this?
@colin99d I could do.
@bolshoytoster I look forward to reviewing it!
@DidierRLopes
We rely strongly on the "slash" separator
Could you direct me to where it's used please, I tried using a regex search but it gave 1712 maches and I don't quite fancy going through that many.
Yeaaa, I figured. Don't worry with it, I think me and @colin99d can handle this from our side.
I think the main things to fix are:
- parents_classes
- scripts folders
- routines folders
- call_exe and call_sh funds that have the hacky version to parse the "/" - there's prob 1 more functionality like this
This will be checked on next major release.