Skip to content
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

Proper user configurable jsk py dict vars #199

Open
wildanrfq opened this issue Feb 10, 2023 · 1 comment
Open

Proper user configurable jsk py dict vars #199

wildanrfq opened this issue Feb 10, 2023 · 1 comment
Labels
no acceptable impl no known implementation that suitably satisfies the conditions of closure within standards

Comments

@wildanrfq
Copy link

The Problem

In the jishaku python's command, there's some aliases we can use to shorten our evaluation code, such as me for Context.me, get for discord.utils.get, etc.

I wonder if configurable aliases like those is possible to implement.

The Ideal Solution

My idea is to add more aliases to jishaku. Something like this design is what I'm thinking of:

import jishaku, discord

jishaku.EvalAliases.add(
  [
    "Embed", discord.Embed,
    "ref", ctx.message.reference.resolved,
  ]
)

The Current Solution

Fork the repository and add the desired aliases manually into the get_var_dict_from_ctx function in repl_builtins.py file.

Summary

A feature that allows the user to configure or add jsk py aliases to use in the command.

I don't know if this feature is already possible to do or implemented, so I'm making an issue to make sure.

@scarletcafe
Copy link
Owner

The problem with this implementation in particular is that it isn't reload-safe. Technically speaking jishaku.Flags isn't fully reload-safe either, but if you use the environment variables you can create persistent settings anyway.

The approaches to dynamic config like this I have in my head are:

  • Letting discord.py handle it (I've requested this before but nobody could ever agree on a good implementation - and it's not hard to see why)
  • Rigging jishaku reload or something to carry settings over - this is not ideal because settings will not survive a full unload-load, it wouldn't survive a reload by any non-jishaku means, and in the case of using things like config classes, would result in old instances of jishaku being persisted in memory due to lingering references
  • Making people use a bot attribute (e.g. bot.jishaku_config) - this kind of duck typing could be hard for linters and things like pylance to understand, and if it needs a config class instance, then it results in the same lingering reference problem as above. The best I could probably do is have jishaku expose a TypedDict and then allow you to set these things through that instead, but I think this approach just sucks for ergonomics in general (Where would a user traditionally put such an assignment? In their bot class? What if they don't have one? What if they need a really large config and want separation of concerns between the configuration and their other stuff?)
  • Attaching the config somewhere where it would silently live for the interpreters lifetime, such as putting it into sys.modules - super ultra gross, this is a real thing that libraries do sometimes and Python itself used to do it for systems without threading support, I just think it's real nasty
  • Making people create some kind of special optional file (e.g. ~/.config/jishaku.toml) - this approach is good for separation but kind of not obvious and "acts at a distance", behavior between users is now dependent on something that is not easy to find and that the user has to explicitly know and remember about, config wouldn't be version controllable together with its bot, what if people want different configs for different bots on the same machine, what if the config needs to be 'rich' (e.g. contain python functions), etc

Ultimately all of these approaches suck in some way that's blocked me from really doing this in the long run - better configuration is something heavily requested and I've wanted to add, but I need to think it over and try to minimize the footguns involved in implementing it.

@scarletcafe scarletcafe added the no acceptable impl no known implementation that suitably satisfies the conditions of closure within standards label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no acceptable impl no known implementation that suitably satisfies the conditions of closure within standards
Projects
None yet
Development

No branches or pull requests

2 participants