Desired features from fork?

[kangalio]

In a fork of this repository, I have implemented multiple dozens of changes and fixes. It would be cool to get a return back on the work put into this, so I'd be quite happy if some of that work was merged into this repo.

The current maintainers expressed that they are only willing to accept a PR with collective changes if I create a list of changes for them to pick from, so here it goes:

1. Godbolt command (this one may already be going to be merged via Add Godbolt Command #77)
2. Miri command
3. Clippy command
4. Macro expansion command
5. Run rustfmt on macro expansion command output: Rust's raw macro expansion output is borderline unviewable due to its absolutely messed up formatting. Attempting to run rustfmt on its output makes the output so much nicer to look at
6. Make fn main optional in ?play
7. Remove ?playwarn; instead automatically showing stderr when it contains useful info
8. Syntax highlighting for playground command responses
9. Make ?eval dynamically behave like ?play when a user accidentally included fn main: this behavior is more streamlined for the user than having to switch commands
10. Strip boilerplate output from playground commands (think Compiling playground, To learn more, run the command again with --verbose., etc.)
11. Remove "compilation succeeded" message with a blank code segment
12. Move crate-attributes in eval-like code to the correct place, instead of throwing a compilation error
13. Correctly query crates.io: Previously, when you would do ?docs serde::something, the bot would search for a crate named serde::something - which will obviously produce garbage results
14. Fix clippy lints
15. Replace all useless instances of pub(crate) with pub
16. Replace the heavy state machine argument parser with a more lightweight, easy to use and flexible argument parser
17. Print a Discord reply when an error occurs during command execution
18. Use &Args everywhere: previously, some places used Args while other used &Args. The inconsistency can trip you up
19. Inline superfluous HOUR constant: it was only used in a single place, and the indirection caused by this constant were causing more reading difficulties than it solved
20. Remove the distinction between SendSyncError and Error and replace all remaining instances of Box<dyn std::error::Error> with the designated type aliases
21. Add ?cleanup command to delete lengthy bot output
22. Broadcast typing animation on command execution: the Playground has relatively long response times, in the realm of 3+ seconds. By broadcasting the typing information, users get immediate feedback for commands, which is more pleasant and user-friendly

[tinaun]

(note: i am not actually an official maintainer of rustbot)

• 1-5 seems good, i want to adjust some of the structure of Add Godbolt Command #77 to deal with point 1 but i should get to it today
• 22 also seems like a no-brainer, as well as 17
• 13 is fixed in some more minor fixes #76
• I kinda like ?play and ?eval being seperate commands but people do get them mixed up a bunch, i could go either way on this.
• I explicitly decided to not syntax highlight command output, because (at least for ?play and ?eval) the output isn't usually rust code, but it makes sense for godbolt and "expand macros", etc
• for 14 i definitely want to fix clippy lints + have ci run clippy on commit tests and not approve commits if there is a clippy issue, but thats up to technetos
• i like keeping the awkward state machine, its fun (but any internal changes should be up to technetos to decide)

[kangalio]

Thank you for the nice feedback. Let's see what technetos or Khionu have got to say about it

[technetos]

The current maintainers expressed that they are only willing to accept a PR with collective changes if I create a list of changes for them to pick from, so here it goes:

That was a miscommunication, whatever changes you want merged from this list need to be separate PRs.

4. Macro expansion command

Totally, would love to see a PR for this

5. Run rustfmt on macro expansion command output: Rust's raw macro expansion output is borderline unviewable due to its absolutely messed up formatting. Attempting to run rustfmt on its output makes the output so much nicer to look at

This could be part of the PR for 4

8. Syntax highlighting for playground command responses

I agree with @tinaun this is kind of excessive for all outputs that are not necessarily rust code. But for godbolt and for macro expansion I think its a great idea.

14. Fix clippy lints

Sure! Please make a PR for this, you could combine this with the changes to have clippy lint on CI or not.

16. Replace the heavy state machine argument parser with a more lightweight, easy to use and flexible argument parser

Heh, I like the state machine, it works quite well and we can do some interesting things with it.

17. Print a Discord reply when an error occurs during command execution

Every error should not necessarily get propagated to the user. This is slightly security related and merits a longer investigation.

18. Use &Args everywhere: previously, some places used Args while other used &Args. The inconsistency can trip you up

Args is passed into the commands by move to allow the commands to do whatever they want with Args. The other places where we pass Args by reference are pretty much all functions designed to be called from within a command that owns Args.

19. Inline superfluous HOUR constant: it was only used in a single place, and the indirection caused by this constant were causing more reading difficulties than it solved

I like to keep constants together, the jobs thread is likely to expand in the future and having the constants in a single place will be good. Additionally, I prefer constants over magic numbers sprinkled in the code.

21. Add ?cleanup command to delete lengthy bot output

This is a neat idea, since users cant delete their own messages, I suppose its not possible to delete the command output either.

Id like to explore this alittle more but if you have a PR ill happily take a look!

22. Broadcast typing animation on command execution: the Playground has relatively long response times, in the realm of 3+ seconds. By broadcasting the typing information, users get immediate feedback for commands, which is more pleasant and user-friendly

I prefer sending a temporary message, then updating it with the results.

[khionu]

Is there any update on this?

[kangalio]

No, I've given up on the proposal because of the immense amount of work required: "whatever changes you want merged from this list need to be separate PRs."