add `mainClient` and `getClient` macros; introduce helper procs; closes #112

[nayr7]

This patch introduce helper procs for dimscord, allowing users to interact with the restapi in a more concise and intuitive way.

The highlights of this PR are the new mainClient and getClient macros that binds against an instance of DiscordClient available at instantiation instead of storing a ref into every object or relying on global variables. However this approach has some pros and cons compared to https://github.com/krisppurg/dimscord/pull/113 and https://github.com/krisppurg/dimscord/pull/101.

Pros:

• No extensive modification of the library existing code is required and no breaking changes.
• Safer than using global variables: the bindsym'ed client is consumed directly and does not leak; getClient will throw a compile-time error if a registered instance of DiscordClient is not in scope, ensuring nil-safety.

Cons:

• template must be used instead of proc in order to implement the helper procs, while this is not a big issue for users, this has led to some degradation in the quality of error messages during my tests.
• Unlike #113 and #101, users must have an instance of DiscordClient around anytime the helper procs must be used which is a very slight con compared to the benefits it brings but still worth mentioning.

To organize theses additions, a new helpers folder is introduced which is include'd in helpers.nim.

TODO:

• [x] resolve naming issues
• [x] add missing helpers
• [x] edit/add tests and ensure correctness of the helper procs
• [ ] edit/add documentation

[nayr7]

I added a TODO to the OP

[nayr7]

About renaming dimClient to useHelpers, I am agaisnt it because we can use it for other purposes than just enabling helper procs. I can technically use it in order to add asserts that check if DiscordClient have the propers intents before calling a proc that would require it which is something me and amadan thought would be nice to have. So it would be wise to name it after what the macro actually does instead of naming it after what we think users would want to use it for

[nayr7]

ah yes, total oversight but please check out the new mention proc in helpers.nim !

[krisppurg]

About renaming dimClient to useHelpers, I am agaisnt it because we can use it for other purposes than just enabling helper procs. I can technically use it in order to add asserts that check if DiscordClient have the propers intents before calling a proc that would require it which is something me and amadan thought would be nice to have. So it would be wise to name it after what the macro actually does instead of naming it after what we think users would want to use it for

Well, first if we are going to check if client has the right intents then the procs should have the client parameter with them instead. I don't think checking intents is a bad idea at all, its just having a limit to only just one discord client registered is the problem I have.

And also I don't think the name dimClient is good, it should just be discordClient or mainClient since we are interacting with discord's api not the library.

[nayr7]

having a limit to only just one discord client registered is the problem I have.

Sorry I dont understand why it's a problem to only have one registered client and how it relate to having a client parameter in the helper procs at all

it should just be discordClient or mainClient since we are interacting with discord's api not the library.

Fair enough, i'll go for mainClient then if you and @ire4ever1190 is fine with it

[ire4ever1190]

mainClient sounds good :+1:

[krisppurg]

having a limit to only just one discord client registered is the problem I have.

Sorry I dont understand why it's a problem to only have one registered client and how it relate to having a client parameter in the helper procs at all

it should just be discordClient or mainClient since we are interacting with discord's api not the library.

Fair enough, i'll go for mainClient then if you and @ire4ever1190 is fine with it

Sorry about that. I really don't want to drag this PR any further about design choice in general and stuff like that.

Here are the reason why I think it's a terrible idea

• If some people start using the pragma and they have other DiscordClients, they would have the intents checker would only be checking the one that has the pragma assigned to it and not the others.
• Having a limit to one registered client is just adding limitations just for the sake of simplicity and enforcing it (when the examples get updated with the helper function). It's the same issue as to what @ire4ever1190 was saying when you used the global var approach with client.

Aside from multiple client talk, I guess {.mainClient.} is alright.

[ire4ever1190]

Bit different to the global var since this enforces only a single instance of the client (If wanting to use the helpers) which needs to be explicitly opt-in. This also requires less rewriting of the library to use inheritance

Couldn't the non helper versions of the procs check intents anyways?

[nayr7]

I really don't want to drag this PR any further about design choice in general and stuff like that. ... If some people start using the pragma and they have other DiscordClients, they would have the intents checker would only be checking the one that has the pragma assigned to it and not the others

We can always discuss this in another PR focusing on this particular issue, I agree that we need to finish this first

Having a limit to one registered client is just adding limitations just for the sake of simplicity and enforcing it (when the examples get updated with the helper function)

But I didn't add those limitations just for simplicity's sake. I did it because misusing something like that was a major concern in my past PRs and this effectively eliminate the risk at compile-time. Another reason is that the syntax of the helper procs becomes less noisy and much more concise (no need to pass a discord.api or ctx and procs act directly on a variable due to UFCs).