Conversation
This adds plover.dev-with-plugins, which is plover.dev with the added plover-plugins-manager dependency (which causes the plugin manager to appear in the Plover interface). Fixes NixOS#89341 in the first way; the second way is morally better but more difficult to do.
ofborg
bot
added
10.rebuild-darwin: 1-10
| src = fetchFromGitHub { | ||
| owner = "ross"; | ||
| repo = "requests-futures"; | ||
| rev = "d5cbf487c010dd84c4e030e279d48e7179e35335"; |
There was a problem hiding this comment.
Please either fetch a tag or change the version to unstable-XXXX-XX-XX.
| pname = "plover-plugins-manager"; | ||
| version = "0.5.16"; | ||
|
|
||
| meta = with lib; { |
There was a problem hiding this comment.
Meta belongs to the end.
| meta = with lib; { | ||
| description = "OpenSteno Plover stenography software plugin manager"; | ||
| maintainers = with maintainers; [ tckmn ]; | ||
| license = licenses.gpl2; |
| }; | ||
|
|
||
| # apply nix sys.path wrapper to plugin manager's python call | ||
| postPatch = "patch -p1 <${./plugins-manager.patch}"; |
There was a problem hiding this comment.
| postPatch = "patch -p1 <${./plugins-manager.patch}"; | |
| postPatch = '' | |
| patch -p1 <${./plugins-manager.patch} | |
| ''; |
Why are you not using the patch in patches? It should support all options.
lambdadog
left a comment
lambdadog
left a comment
There was a problem hiding this comment.
I think there are a couple of important changes to be made here, but I'm happy to see some progress made on this issue after all this time!
| propagatedBuildInputs = [ pip pkginfo dev pygments readme_renderer requests requests-cache requests-futures setuptools wheel ]; | ||
| }; | ||
|
|
||
| dev-with-plugins = dev.overrideAttrs (old: { |
There was a problem hiding this comment.
Couple of thoughts:
Recommend adding a withPluginManager passthrough to the dev attribute and having this point to it. This allows one to override plover.dev if they desire and still have access to withPluginManager behavior without having to create an overlay (through (plover.dev.overrideAttrs (old: ...)).withPluginManager)
Also I think it's probably confusing to have a withPackages-sounding attribute in the packageset be an attribute that doesn't take an argument (namely, a list of the plugins). I notice that you used kebab-case to name it, which disambiguates it a little from functions like emacsWithPackages but personally I'd like to see 1 of 3 things:
- Making it crystal clear exactly what this is -- ie. calling it something like
dev-with-plugin-manager - Just adding
plugin-managertodev(making it disableable with an override, perhaps)- Honestly I think this is the behavior most people would be expecting by default anyway, since on every other platform I can think of, when you install plover dev, the plugin manager comes with.
- Implementing
emacsWithPackages-like functionality
There was a problem hiding this comment.
I've distilled out my thoughts on this and I'm pretty sure the second option is the right way to do this. Plover coming with the plugin manager is expected behavior when installing Plover, so it's reasonable to have it be the default.
Ideally it would be disable-able by overriding the package for those who don't want it, and the third option (emacsWithPackages-like functionality) could be implemented later by anyone who felt inclined to do so.
SuperSandro2000
added
the
2.status: merge conflict
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
3 participants
Motivation for this change
This adds plover.dev-with-plugins, which is plover.dev with the added
plover-plugins-manager dependency (which causes the plugin manager to
appear in the Plover interface).
Fixes #89341 in the first way; the second way is morally better but more
difficult to do.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)