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.

Sign up for GitHub

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

libreoffice: Templates for LibreOffice Impress #94753

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

libreoffice: Templates for LibreOffice Impress #94753

wants to merge 3 commits into from

Conversation

LouisDK1
Copy link
Contributor

@LouisDK1 LouisDK1 commented Aug 5, 2020

Motivation for this change

LibreOffice Impress has few templates by default so I want to get some more.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

symphorien
symphorien reviewed Aug 5, 2020
View reviewed changes
pkgs/applications/office/libreoffice/wrapper.nix Outdated
@@ -1,4 +1,4 @@
{ libreoffice, runCommand, dbus, bash }:
{ libreoffice, runCommand, dbus, bash, plugins }:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you add the argument but don't use it...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@symphorien
I know. This is still a draft. I need to figure out how to complete wrapper.nix. If somebody has a suggestion please let me know.

@LouisDK1
Copy link
Contributor Author

LouisDK1 commented Aug 7, 2020

@rycee I'm a bit unsure if I should add the templates as an optional option in default or put it in wrapper.nix as a plugin. What do you suggest?

@LouisDK1
Copy link
Contributor Author

@flokli @rycee I'm a bit unsure if I should add the templates as an optional option in default or put it in wrapper.nix as a plugin. What do you suggest?
If a wrapper would be a good idea I'd like a suggestion as the wrapper for libreoffice seems to differ a bit from other wappers including plugins.

@flokli
Copy link
Contributor

flokli commented Aug 15, 2020

@flokli @rycee I'm a bit unsure if I should add the templates as an optional option in default or put it in wrapper.nix as a plugin. What do you suggest?
If a wrapper would be a good idea I'd like a suggestion as the wrapper for libreoffice seems to differ a bit from other wappers including plugins.

Can you point libreoffice to a certain location to look for templates at all?

What's the size of these templates when built? Given libreoffice itself is ~500MiB, some more MiB probably are fine, if a version without templates can still be built by passing null instead of the templates.

flokli
flokli requested changes Aug 15, 2020
View reviewed changes
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please push a version that builds.

@LouisDK1
Copy link
Contributor Author

LouisDK1 commented Aug 20, 2020 •
edited

@flokli

Can you point libreoffice to a certain location to look for templates at all?

If they're located at "/lib/libreoffice/share/template/" they should be discovered by default if LibreOffice has access to that path.

What's the size of these templates when built? Given libreoffice itself is ~500MiB, some more MiB probably are fine, if a version without templates can still be built by passing null instead of the templates.

It's around ~100MB.

I personally would prefer a package called "libreofficePlugins.impress-templates" making room for more plugins at a later time.

I'm a bit stuck on this as different packages write there plugins.nix in a different way.
I've looked a several plugins.nix (eg. from kodi, deadbeef and chromium) but have yet to find a proper way.

My plan is to do a "callPackage" for libreofficePlugins in either all-packages.nix or default.nix for libreoffice and then the user can specify to build all plugins or just impress-templates and by default none would get build.

A link to an inspirational plugins.nix or a hint in the right direction would be very appreciated.

@LouisDK1 LouisDK1 force-pushed the libreoffice-impress-templates branch 2 times, most recently from a1d4f36 to 38e9172 Compare August 20, 2020 20:08
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/making-plugin-section-for-libreoffice/8676/1

@LouisDK1
Copy link
Contributor Author

@primeos Looking at my comment above would you be able to give me some advice?

@primeos
Copy link
Member

primeos commented Aug 25, 2020 •
edited

@LouisDK1 please don't start to cc me randomly, I can only help if I'm familiar with the things you change. If that was due mentioned Chromium plugins then that makes sense. But I never touched the plugin related stuff. You can use things like the GitHub UI or git shortlog -sne pkgs/applications/office/libreoffice/ (pkgs/applications/networking/browsers/chromium/plugins.nix, etc.) to find people that can potentially help ;)

Edit: But as a hint: If you look at https://github.com/NixOS/nixpkgs/pull/94753/files the parts with a red text background are due to syntax errors.

rycee
rycee reviewed Sep 5, 2020
View reviewed changes
pkgs/applications/office/libreoffice/plugins.nix Outdated
@@ -0,0 +1,37 @@
{ stdenv, fetchFromGitHub, ruby.devEnv, zip }:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll probably need something like

Suggested change
{ stdenv, fetchFromGitHub, ruby.devEnv, zip }:
{ stdenv, fetchFromGitHub, ruby, zip }:

@stale
Copy link

stale bot commented Mar 5, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 5, 2021
@LouisDK1 LouisDK1 force-pushed the libreoffice-impress-templates branch from 6b35026 to 8c04023 Compare May 30, 2021 13:59
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 30, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rycee rycee rycee left review comments

@symphorien symphorien symphorien left review comments

@flokli flokli flokli requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
2.status: merge conflict
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@LouisDK1 @flokli @nixos-discourse @primeos @rycee @symphorien @wegank