Skip to content
This repository has been archived by the owner on Jun 15, 2018. It is now read-only.

Add bash command #41

Merged
merged 3 commits into from
Aug 17, 2017
Merged

Add bash command #41

merged 3 commits into from
Aug 17, 2017

Conversation

spotlightishere
Copy link
Contributor

This PR would fix #38 if merged. It outputs the command in a similar format to eval (saying both input and output) but has a major drawback.

It would appear that Runtime.getRuntime().exec has issues with relative commands. For example, you're unable to bash ls -l ~ but you're able to bash ls /home/spotlight, which I find odd. I have no idea what the issue is and it would appear that Google doesn't either.

Special thanks to @ArtutoGamer for helping me out with this as well. This PR is 10000x better than #39 😅

@jagrosh
Copy link
Owner

jagrosh commented Jul 29, 2017

Since this prints error traces to console, maybe it should print results that are too long to console as well. Alternatively, it could upload a txt file with the results

try {
reply("Input: ```\n" + args + "``` Output: \n```\n" + finalOutput + "```", event);
} catch (IllegalArgumentException e) {
reply("Command output too long!", event);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make it so it prints it to console, similar to how it does on an exception?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I can do it later if Spotlight can't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome to, I've been busy due to personal reasons and have yet to mess with it.

@spotlightishere
Copy link
Contributor Author

After messing around with the command for a bit, I found out that Runtime.getRuntime().exec halts the program if anything is sent to stderr for the error's InputStream to be read. Instead of finding some way to incorporate the two, I switched to using ProcessBuilder instead.

See also, https://stackoverflow.com/a/23878137/3874884

(Also, I added in outputting to console. Uploading a file wouldn't be fun as we'd have to check permissions and blek.)

@ajloveslily14
Copy link

What about instead of uploading a file it just posts to pastebin or something like that? Would there have to be a perm check for the file upload? Wouldn't it just fail silently?

@spotlightishere
Copy link
Contributor Author

spotlightishere commented Aug 13, 2017

Doing pastebin could work indeed, though we'd have to bring up an API and even then I'd rather use Ghostbin (or a user might want to use Hastebin, etc). If jag wants it I could add it, otherwise output to the console is good for now.

@Artuto
Copy link

Artuto commented Aug 14, 2017 via email

@ajloveslily14
Copy link

ajloveslily14 commented Aug 14, 2017 via email

@jagrosh
Copy link
Owner

jagrosh commented Aug 17, 2017

Gonna merge this now, might be a good idea to add the txt file upload at some point in the future though

@jagrosh jagrosh merged commit 5906eff into jagrosh:master Aug 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Suggestion] Bash command
4 participants