-
-
Notifications
You must be signed in to change notification settings - Fork 89
Migrate WolframAlpha command #212
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
Conversation
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
.../java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandTest.java
Outdated
Show resolved
Hide resolved
.../java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandTest.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
We had to restrict it to 10 images, because of JDA. |
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
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.
In my opinion, you should add a lot more empty lines between the code.
It's really hard to read, especially with those if without brackets.
I added an example, of how it'd be more readable in my opinion.
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
9da377b
to
9ca2785
Compare
application/src/main/java/org/togetherjava/tjbot/commands/utils/WolfCommandUtils.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
We will fix the design issues later, now I feel we should move on to making the error POJOs |
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
Keep in mind that the API is quite buggy. Sometimes if you make a spelling error it will send a tip asking you to spell correctly, at other times it will just auto correct it and sometimes it will send a |
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
...ation/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolfImage.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/ExpressionTypes.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/DidYouMeans.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/QueryResult.java
Outdated
Show resolved
Hide resolved
...ation/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolfImage.java
Outdated
Show resolved
Hide resolved
c328402
to
d4dbe08
Compare
d4dbe08
to
2acb1c3
Compare
...ava/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/misunderstoodqueries/Tips.java
Outdated
Show resolved
Hide resolved
...ation/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Constants.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/RelatedExamples.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Tips.java
Outdated
Show resolved
Hide resolved
...main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommand.java
Outdated
Show resolved
Hide resolved
...java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandUtils.java
Outdated
Show resolved
Hide resolved
...java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandUtils.java
Outdated
Show resolved
Hide resolved
...java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandUtils.java
Outdated
Show resolved
Hide resolved
...java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandUtils.java
Outdated
Show resolved
Hide resolved
...java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/WolframAlphaCommandUtils.java
Outdated
Show resolved
Hide resolved
This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days. |
Hello I am just responding to the review comments. |
65d1e84
to
2141bdd
Compare
taking over |
...ation/src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/Constants.java
Outdated
Show resolved
Hide resolved
Stuff Created a class for the expression types tag. `deferReply` at the start now. Added Tips and Tip classes for the error POJOs Some changes to POJO classes Forgot to include this in the commit used the Tips POJO committing gitignore Changed as per @Zabuzard 's review Performed the changes requested a small change. Using Optionals instead of a try-catch @NotNull added and spotless applied Some changes Added unit test and utility class Some changes tested a failing case which failed successfully and wrote the pixel by pixel check to compare the images. Another test Fixed `combineImages` A different approach to send the images. removed a single `.` Logged stuff for debugging. Some changes Fixed some issues and removed some obsolete logs. Fixed almost everything but might change entire design Embeds Working embeds Alternative method using `WebhookMessageUpdateAction` New Package Did you means Future topics, Language messages and Related Examples Example Page This did not get committed properly Updating QueryResult Sending the actual URI to the users now Query result fix and displaying an error if it occurs @JsonProperty @JacksonXmlElementWrapper POJOs fix Added some examples POJO fix0 POJO fix1 POJO fix2 POJO fix3 POJO fix4 POJO fix5 POJO fix6 POJO fix7 POJO fix8 Handling the unsuccessful result case Added Error POJOS Some refactoring The result of some git forkery Ready for review :thumbs_up ENUM!!!!!! Some more refactoring Fixed overlapping "error" attribute conflict Error unrapping Refactoring... Some more refactoring remove import Remove that method I am back Fix config issue Put constants in seperate enum Put constants in seperate enum Some minor changes Some more minor changes even less major changes Better UX Fix mistake Fix rebasing issues. fix that issue remove comment
095e2f0
to
7c6a7c0
Compare
215f78b
to
09a7e6c
Compare
b616629
to
c1da29b
Compare
...src/main/java/org/togetherjava/tjbot/commands/mathcommands/wolframalpha/api/QueryResult.java
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
This adds the WolframAlpha command (previously known as
=wolf
). Command example:and the bot will respond with rendered images and data by using the WolframAlpha API.
Details
The code is straightforward:
The image fiddling can be a bit awkward but ultimately its quite simple. The reason why we do not just send one big picture is because Discord would otherwise scale it down and it would not provide a nice UX. So instead we send multiple smaller images, so that Discord doesnt make them smaller.
Discord has a limit of 10 embeds, so we will cut off the rest in case its too much. Also, there are other edge cases leading to only partial results (for example if pods timed out).
In general, the response always consists of a quick summary/explanation (also with error messages, if any) and then the embeds.
Embeds itself can display images if they have been added to the message as attachment.
Also note that Wolfram Alpha itself also provides some error messages with extra info, which we can display as well:
Config
We have created a community account and created two application IDs:
master
), details are in the moderator chat (do not share)develop
), which is79J52T-6239TVXHR7
.The bot picks it up from the
config.json
file. Therefore, a new parameter has been added. Adjust your config file and add: