I wouldn't mind being marked as a maintainer (maintainers.bb010g).

The ptb and canary branches now work as well if they're selected.

Also adressed other review comments, and updated to 1.7.0.

Does this create a separate betterdiscordctl package for each version of Discord?

It does not, it allows you to select which branch to use with the branch variable (default is stable).
That sets the discord variable to the version of Discord selected, which gets used in the patchPhase.

I'm not familiar with nix packaging. Can you change branches on the fly or do you have to reinstall it in order to use it on another Discord branch?

You'd have to reinstall as it needs to repatch. Not sure of a better solution. However as 99.99% of users just use 1 branch of Discord i dont think it should be much of an issue.

How about patching it so that it prefixes /opt with ${pkgs.discord} or the corresponding flavor selected by the --flavor flag on runtime instead? This should probably be handled upstream by adding a --nix flag (akin to bb010g/betterdiscordctl#59) and enabling it by default in the nix package.

Sure, sounds good. If someone merges this (The installation derivation from that PR wouldn't be required as we handle that here) we can just wrap it to automatically use that flag here. Would indeed be a bit cleaner.

Edit: Oh, looks like that would only support the stable branch, as other binaries are named differently. Would require a bit more than just merging that.

The PR implementation doesn't seem like it'll work with PTB or Canary either.
Anyway, I'll let @bb010g handle the rest since I'm way out of my depth here.

Any update on this? If not i think what I've done currently should be fine. I forgot to add documentation on how to switch discord versions in the package, so I'll add that now.

I made an attempt over at bb010g/betterdiscordctl#67.

If it's merged, patchPhase can be replaced with:

substituteInPlace betterdiscordctl --replace "^DISABLE_UPGRADE=$" "DISABLE_UPGRADE=yes"
substituteInPlace betterdiscordctl --replace "^nix=$" "nix=yes"

(Assuming regular expressions are supported.)

Also, the version should be changed to 1.7.1 and the branch derivation won't be needed.

Since it's taking a while for that PR to get merged upstream, I've just patched it in here. Current derivation works just fine, and is a lot cleaner than before IMO :)

Should be ready to merge.

Also, the version should be changed to 1.7.1 and the branch derivation won't be needed.

That release does not seem to exist?

That release does not seem to exist?

Yeah, I was going to publish it once the PR was merged.

is this ready? if the latest review is blocking then you could use sed right?

is this ready? if the latest review is blocking then you could use sed right?

I've done that, should indeed be ready now 👍

Sorry about the wait, totally forgot about this one.

Result of nixpkgs-review pr 86943 1

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/312

Result of nixpkgs-review pr 86943 run on x86_64-linux 1

@IvarWithoutBones v1.7.1 has been published now. (Though BD broke in the meantime.)