unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Marius Bakke <mbakke@fastmail.com>
To: "José Miguel Sánchez García" <jmi2k@openmailbox.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: Add vis, libtermkey and lua-lpeg.
Date: Thu, 08 Dec 2016 15:17:59 +0100	[thread overview]
Message-ID: <8737hyzeqw.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me> (raw)
In-Reply-To: <dfce56f98ee68c2f45f67b03c87076da@openmailbox.org>

[-- Attachment #1: Type: text/plain, Size: 2969 bytes --]

José Miguel Sánchez García <jmi2k@openmailbox.org> writes:

> On 2016-12-07 23:23, Marius Bakke wrote:
>> José Miguel Sánchez García <jmi2k@openmailbox.org> writes:
>> 
>>> Let's see if these patches get accepted now! I think I've taken into
>>> account
>>> every single detail.
>>> 
>>> These patches add three packages: vis, lua and libtermkey. vis depends
>>> on lua
>>> and libtermkey, so add them before adding vis.
>>> 
>>> vis is being added in a new file, gnu/text-editors.scm , as requested
>>> here
>>> <https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00242.html>
>> 
>> Thanks for these patches!
>> 
>> I've committed the first two packages with some minor edits:
>> 
>> * Indentation ran through emacs (e.g. M-x indent-region). We follow
>>   emacs indentation rules religiously. :-)
>> * Updated commit messages to match the GNU changelog style.
>> * libtermkey and lua-lpeg actually had tests. 'gnu-build-system' runs
>>   "make check" by default, whereas they expected "make test". This can
>>   be overridden with the #:test-target argument.
>> * Avoided hard coding the lua version in lpeg install path.
>> 
>> 'vis' was a little more tricky. On startup it could not find the 
>> "visrc"
>> file, which you probably had in ~/.config or similar. I solved that by
>> adding a "native-search-path" for VIS_PATH so Guix sets up this 
>> variable
>> when installed (this seems to be an undocumented feature of Guix :-)).
>> 
>> In a perfect world, the same trick could be used for LUA_PATH and
>> LUA_CPATH to discover the optional "lpeg" module, but they don't seem 
>> to
>> behave like normal PATH specifications and requires a pattern match,
>> which in turn appears to get ignored by guix' 
>> search-path-specification.
>> 
>> Instead I wrapped the binary with the lpeg paths. This also seems to
>> have fixed the problem finding themes. Can you try the attached patch
>> and see if that works for you, especially if you have a custom config?
> I'm in a VM, so my home directory is pretty much empty, but you're 
> right: weeks
> back when I was trying to get it working on my machine I created an 
> empty
> config file, so I forgot about it when I submitted the patches.
>
> I tested it, and it works fine. I'll take a look at the modifications 
> you did
> to learn more about what to do for future patches.
>
> Finally, thanks for finally accepting these packages. I worked really 
> hard to
> prepare them (I didn't know any Scheme before starting!), but the 
> community has
> proven to be very helpful :D.

Thanks for checking (it seems you forgot to CC the list) :-)

I tested it with a custom config just now, but it seems to load the one
from VIS_PATH before checking ~/.config. I'm not sure how to fix that
without breaking the lexer integration, so I've committed it as-is.
Would be nice to have a fix for that though ;-)

Thanks again for contributing!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

      parent reply	other threads:[~2016-12-08 14:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-07 17:33 [PATCH] gnu: Add vis, libtermkey and lua-lpeg José Miguel Sánchez García
2016-12-07 22:23 ` Marius Bakke
     [not found]   ` <dfce56f98ee68c2f45f67b03c87076da@openmailbox.org>
2016-12-08 14:17     ` Marius Bakke [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8737hyzeqw.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me \
    --to=mbakke@fastmail.com \
    --cc=guix-devel@gnu.org \
    --cc=jmi2k@openmailbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).