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

w3m and ranger fixes #11222

Merged
merged 2 commits into from Nov 28, 2015
Merged

w3m and ranger fixes #11222

merged 2 commits into from Nov 28, 2015

Conversation

oxij
Copy link
Member

@oxij oxij commented Nov 23, 2015

Rebase of #10995 onto staging

@oxij oxij mentioned this pull request Nov 23, 2015
@jagajaga
Copy link
Member

cc @edolstra

graphicsSupport = false;
x11Support = false;
mouseSupport = false;
Copy link
Member

Choose a reason for hiding this comment

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

@oxij awesome work.

i have only once comment just to keep things consistent with other derivations. usually we separate a package into and Full ... example (python and pythonFull). i think here we should do the same and have

w3m be what currently is w3m-pure and create new w3mFull which would bring all the graphicSupport, x11Support, mouseSupport.

basically all i would like is to things be more or less consistent. @oxij you should wait with any changes until somebody else comments and confirms this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, it is more consistent, consistent is good, I can do that. But even
then I'd still like to have an explicitly minimal version in case
someone else will fiddle with defaults in the future. w3m-pure is for
derivations that use w3m in batch mode for text rendering, not browsing,
and I'd like that information to be explicit in nixpkgs. Which means
there would be three w3m's: w3m, w3mFull, w3mMinimal (current
w3m-pure) and

  • I have no idea what should be the difference between w3m and
    w3mFull if the default derivation should be what "most users will want
    by default".
  • ranger will have to use w3mFull or specifically crafted version (4th
    w3m!) with image support.

So, should I rename w3m-pure to w3mMinimal and define w3mFull = w3m?

(Btw, I still think that using nixpkgs.config options (constantly
rejected in nixpkgs) is a good thing as it simplifies things a lot, e.g.
it can simplify these four w3ms into two and one assert in ranger,
but whatever.)

vcunat pushed a commit that referenced this pull request Nov 23, 2015
Picked from #11222.

(cherry picked from commit b13c718)
vcunat pushed a commit that referenced this pull request Nov 23, 2015
@vcunat
Copy link
Member

vcunat commented Nov 23, 2015

I picked the ranger commit, as that one certainly doesn't need to go through staging. And it looks fine, thanks.

@nbp nbp added this to the 16.03 milestone Nov 23, 2015
@vcunat vcunat mentioned this pull request Nov 24, 2015
meta = with stdenv.lib; {
postInstall = optionalString x11Support ''
rpath=`patchelf --print-rpath $out/libexec/w3m/w3mimgdisplay`
patchelf --set-rpath "$rpath:${libX11}/lib:${libX11}/lib64" $out/libexec/w3m/w3mimgdisplay
Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid this by using:

  LIBS = lib.optionalString graphicsSupport "-lX11";

The autoconf stuff will pick up that environment variable.

@cstrahan
Copy link
Contributor

I think my PR (#11236) subsumes much of this, with the exception of touching the names in all-packages.

Regarding the names, I'd suggest having w3m be minimal, and have w3mFull (or something along those lines) be w3m with all of the bells and whistles.

I would suggest pulling in my PR, and then layering on these changes. How does that sound?

@oxij
Copy link
Member Author

oxij commented Nov 26, 2015 via email

@oxij
Copy link
Member Author

oxij commented Nov 28, 2015 via email

jagajaga added a commit that referenced this pull request Nov 28, 2015
@jagajaga jagajaga merged commit 8795eeb into NixOS:staging Nov 28, 2015
@magnetophon
Copy link
Member

Great, I was just about to work out proper img-previewing in ranger; thanks!
Would it make sense to backport to 15.09 as well?

PS: I fixed the default image previewing as well.

@oxij
Copy link
Member Author

oxij commented Nov 29, 2015 via email

@cstrahan
Copy link
Contributor

Great, I was just about to work out proper img-previewing in ranger; thanks!

I find it interesting that we were looking into this at roughly the same time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants