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

Jump to bottom

urxvt_bidi: init at 2.15 #70312

Merged
merged 2 commits into from Nov 1, 2019
Merged

urxvt_bidi: init at 2.15 #70312

merged 2 commits into from Nov 1, 2019

Conversation

doronbehar
Copy link
Contributor

Motivation for this change

Add a very useful plugin to urxvt - https://github.com/mkamensky/Text-Bidi .

Things done

All things done below were tested with #70310 merged, hence it's a requirement for this to get merged.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @rnhmjoj as you are the maintainer of urxvt.

@doronbehar
urxvt_bidi: init at 2.15
e21a1fd 
Add plugin to default wrapper.
doronbehar added a commit to doronbehar/.config_nixpkgs that referenced this pull request Oct 3, 2019
@doronbehar
Add urxvt_bidi plugin
eb2d761 
See NixOS/nixpkgs#70312
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Oct 3, 2019

Thank you, I've just tested and it works. Everything looks good.
I'm not sure about enabling it by default, the closure increase is pretty small (≈1MB), though.

@doronbehar
Copy link
Contributor Author

TBH I wouldn't care much, it's just that it's a little bit nontrivial for this package to be added to the plugin's list because of the perlDeps which needs to have urxvt_bidi as well. I wanted to show for anyone who may read the definition of rxvt_unicode-with-plugins and that may want to specify his own plugins in his config, that this is needed.

Would you be satisfied if I would leave an appropriate comment instead?

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Oct 4, 2019

TBH I wouldn't care much, it's just that it's a little bit nontrivial for this package to be added to the plugin's list because of the perlDeps which needs to have urxvt_bidi as well.

Yes, definitely: I intend to rewrite the plugin system to handle dependencies for this reason.

Would you be satisfied if I would leave an appropriate comment instead?

No, you're right: It's better this way until there is a way for a plugin to export perl dependencies, not many people look at the source code anyway.

@doronbehar
Copy link
Contributor Author

Oh right so I'll leave it as is. Do you have write permissions to the nixpkgs?

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Oct 4, 2019

Oh right so I'll leave it as is. Do you have write permissions to the nixpkgs?

No, sorry. I'll try to get attention.

doronbehar added a commit to doronbehar/.config_nixpkgs that referenced this pull request Oct 12, 2019
@doronbehar
Add urxvt_bidi plugin
8072d5e 
See NixOS/nixpkgs#70312
@nixos-discourse
Copy link

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

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

@doronbehar doronbehar mentioned this pull request Oct 24, 2019
Merged
10 tasks
@veprbl
Copy link
Member

veprbl commented Nov 1, 2019

@GrahamcOfBorg build rxvt_unicode-with-plugins urxvt_bidi

@veprbl veprbl requested a review from volth November 1, 2019 13:38
@veprbl veprbl merged commit cf2a4ff into NixOS:master Nov 1, 2019
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jun 16, 2020
@doronbehar @dtzWill
urxvt_bidi: init at 2.15 (NixOS#70312)
b6e3fc9 
Add plugin to default wrapper.

(cherry picked from commit cf2a4ff)
@doronbehar doronbehar deleted the package-urxvt_bidi branch March 2, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnhmjoj rnhmjoj rnhmjoj approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@doronbehar @rnhmjoj @nixos-discourse @veprbl