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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion nixos/modules/services/misc/nixos-manual.nix
Expand Up @@ -117,7 +117,7 @@ in
services.mingetty.helpLine = mkIf cfg.showManual
"\nPress <Alt-F${toString cfg.ttyNumber}> for the NixOS manual.";

services.nixosManual.browser = mkDefault "${pkgs.w3m}/bin/w3m";
services.nixosManual.browser = mkDefault "${pkgs.w3m-nox}/bin/w3m";

};

Expand Down
36 changes: 22 additions & 14 deletions pkgs/applications/networking/browsers/w3m/default.nix
@@ -1,16 +1,18 @@
{ stdenv, fetchurl
, sslSupport ? true
, graphicsSupport ? false
, mouseSupport ? false
, ncurses, openssl ? null, boehmgc, gettext, zlib
, imlib2 ? null, xlibsWrapper ? null, fbcon ? null
, gpm-ncurses ? null
, ncurses, boehmgc, gettext, zlib
, sslSupport ? true, openssl ? null
, graphicsSupport ? true, imlib2 ? null
, x11Support ? graphicsSupport, libX11 ? null
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure that graphicsSupport should imply x11Support, so I don't think we should add this option (unless I'm mistaken).

Copy link
Member Author

@oxij oxij Nov 25, 2015 via email

Choose a reason for hiding this comment

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

Copy link
Member Author

@oxij oxij Nov 25, 2015 via email

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

graphicsSupport thingy in your comment and PR is not okay, because my
patch also works without X11 in frame buffer (I actually use that with
ranger).

Ah, OK. I thought imlib was used only for rendering, but if it serves another text-only purpose, then I agree with your rationale.

I also don't like the idea of w3m == w3m-pure because then ranger,
emacs and everything else that uses w3m as a browser (that is, all
user-oriented stuff) will need to be overriden which is not nice. I can
agree to rename w3m-pure to w3mBatch and define w3mFull = w3m if
that bothers anyone, but nothing less.

Yeah, that is an interesting point. I could see using w3mPure where needed, then one could override w3m to enable whatever they want, and then that would get picked up in reverse dependencies. Sounds good.

I say after I fix LIB thingy, test that it doesn't break the frame
buffer and everyone agrees to these changes, then you should apply your
fetchpatch-changes (on top or in parallel, shouldn't matter, to this).

Sounds good.

Thanks for this PR 🍺

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay then, rebuilding the thing right now (will take a while with gcc update).

I decided to do this:

w3m = callPackage ../applications/networking/browsers/w3m { };

Should always be the version with the most features

w3m-full = w3m;

Version without X11

w3m-nox = w3m.override {
x11Support = false;
};

Version for batch text processing, not a good browser

w3m-batch = w3m.override {
graphicsSupport = false;
x11Support = false;
mouseSupport = false;
};

Does this look good to you?

Thank you, too!

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

w3m-batch is not darwin friendly since w3m refuses to evaluate because of gpm by having mouseSupport set to true by default

hmm I'm not too familiar with nixpkgs, but I'm currently using callPackage instead of override and I guess the same would be required for w3m-nox.

w3m-batch = callPackage ../applications/networking/browsers/w3m {
  graphicsSupport = false;
  x11Support = false;
  mouseSupport = false;
};

Copy link
Member Author

Choose a reason for hiding this comment

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

Does the following help?

diff --git a/pkgs/applications/networking/browsers/w3m/default.nix b/pkgs/applications/networking/browsers/w3m/default.nix
index e71a733..bea74f6 100644
--- a/pkgs/applications/networking/browsers/w3m/default.nix
+++ b/pkgs/applications/networking/browsers/w3m/default.nix
@@ -3,7 +3,7 @@
, sslSupport ? true, openssl ? null
, graphicsSupport ? true, imlib2 ? null
, x11Support ? graphicsSupport, libX11 ? null
-, mouseSupport ? true, gpm-ncurses ? null
+, mouseSupport ? !stdenv.isDarwin, gpm-ncurses ? null
}:

Copy link
Contributor

Choose a reason for hiding this comment

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

nice! that works for me

, mouseSupport ? true, gpm-ncurses ? null
}:

assert sslSupport -> openssl != null;
assert graphicsSupport -> imlib2 != null && (xlibsWrapper != null || fbcon != null);
assert graphicsSupport -> imlib2 != null;
assert x11Support -> graphicsSupport && libX11 != null;
assert mouseSupport -> gpm-ncurses != null;

with stdenv.lib;

stdenv.mkDerivation rec {
name = "w3m-0.5.3";

Expand All @@ -24,16 +26,19 @@ stdenv.mkDerivation rec {
patches = [ ./glibc214.patch ]
# Patch for the newer unstable boehm-gc 7.2alpha. Not all platforms use that
# alpha. At the time of writing this, boehm-gc-7.1 is the last stable.
++ stdenv.lib.optional (boehmgc.name != "boehm-gc-7.1") [ ./newgc.patch ]
++ stdenv.lib.optional stdenv.isCygwin ./cygwin.patch;
++ optional (boehmgc.name != "boehm-gc-7.1") [ ./newgc.patch ]
++ optional stdenv.isCygwin ./cygwin.patch
# for frame buffer only version
++ optional (graphicsSupport && !x11Support) [ ./no-x11.patch ];

buildInputs = [ncurses boehmgc gettext zlib]
++ stdenv.lib.optional sslSupport openssl
++ stdenv.lib.optional mouseSupport gpm-ncurses
++ stdenv.lib.optionals graphicsSupport [imlib2 xlibsWrapper fbcon];
++ optional sslSupport openssl
++ optional mouseSupport gpm-ncurses
++ optional graphicsSupport imlib2
++ optional x11Support libX11;

configureFlags = "--with-ssl=${openssl} --with-gc=${boehmgc}"
+ stdenv.lib.optionalString graphicsSupport " --enable-image=x11,fb";
+ optionalString graphicsSupport " --enable-image=${optionalString x11Support "x11,"}fb";

preConfigure = ''
substituteInPlace ./configure --replace "/lib /usr/lib /usr/local/lib /usr/ucblib /usr/ccslib /usr/ccs/lib /lib64 /usr/lib64" /no-such-path
Expand All @@ -42,7 +47,10 @@ stdenv.mkDerivation rec {

enableParallelBuilding = false;

meta = with stdenv.lib; {
# for w3mimgdisplay
LIBS = optionalString x11Support "-lX11";

meta = {
homepage = http://w3m.sourceforge.net/;
description = "A text-mode web browser";
maintainers = [ maintainers.mornfall ];
Expand Down
15 changes: 15 additions & 0 deletions pkgs/applications/networking/browsers/w3m/no-x11.patch
@@ -0,0 +1,15 @@
Forget about X11 in fb module.
This breaks w3mimgdisplay under X11, but removes X11 dependency it in pure fb.
diff --git a/w3mimg/fb/fb_imlib2.c b/w3mimg/fb/fb_imlib2.c
index ea36637..d3d7bc3 100644
--- a/w3mimg/fb/fb_imlib2.c
+++ b/w3mimg/fb/fb_imlib2.c
@@ -3,7 +3,7 @@
fb_imlib2.c 0.3 Copyright (C) 2002, hito
**************************************************************************/

-#include <X11/Xlib.h>
+#define X_DISPLAY_MISSING
#include <Imlib2.h>
#include "fb.h"
#include "fb_img.h"
26 changes: 22 additions & 4 deletions pkgs/top-level/all-packages.nix
Expand Up @@ -924,6 +924,7 @@ let

asciidoc = callPackage ../tools/typesetting/asciidoc {
inherit (pythonPackages) matplotlib numpy aafigure recursivePthLoader;
w3m = w3m-batch;
enableStandardFeatures = false;
};

Expand Down Expand Up @@ -3143,7 +3144,9 @@ let

stricat = callPackage ../tools/security/stricat { };

privoxy = callPackage ../tools/networking/privoxy { };
privoxy = callPackage ../tools/networking/privoxy {
w3m = w3m-batch;
};

swaks = callPackage ../tools/networking/swaks { };

Expand Down Expand Up @@ -3597,7 +3600,7 @@ let
xmlstarlet = callPackage ../tools/text/xml/xmlstarlet { };

xmlto = callPackage ../tools/typesetting/xmlto {
w3m = w3m.override { graphicsSupport = false; };
w3m = w3m-batch;
};

xmltv = callPackage ../tools/misc/xmltv { };
Expand Down Expand Up @@ -13388,8 +13391,21 @@ let

vym = callPackage ../applications/misc/vym { };

w3m = callPackage ../applications/networking/browsers/w3m {
w3m = callPackage ../applications/networking/browsers/w3m { };

# Should always be the version with the most features
w3m-full = w3m;

# Version without X11
w3m-nox = w3m.override {
x11Support = false;
};

# Version for batch text processing, not a good browser
w3m-batch = w3m.override {
graphicsSupport = false;
x11Support = false;
mouseSupport = false;
};

weechat = callPackage ../applications/networking/irc/weechat {
Expand Down Expand Up @@ -13592,7 +13608,9 @@ let

xdg-user-dirs = callPackage ../tools/X11/xdg-user-dirs { };

xdg_utils = callPackage ../tools/X11/xdg-utils { };
xdg_utils = callPackage ../tools/X11/xdg-utils {
w3m = w3m-batch;
};

xdotool = callPackage ../tools/X11/xdotool { };

Expand Down