[new port] x11/lilyterm: Lightweight, but functional terminal emulator

[jwb]

Summary

[new port] x11/lilyterm: Lightweight, but functional terminal emulator

Approved by: jrm (mentor) or wen (mentor)
Differential to be added to commit log

Interesting case in light of our discussion about PORTNAME. I tried changing
it to LilyTerm (to match the Github project name) and discovered that doing so
would necessitate internal patching to change the default install directories.
(DOSCDIR and EXAMPLESDIR). So I reverted to lilyterm to reduce complexity.
Also opted not to include DOCS and EXAMPLES as options since they only
install 4 files combined.

Test Plan

portlint -C: Looks fine
Poudriere tested on {10.3,11.1}-{amd64,i386}

[mat]

x11/lilyterm/Makefile

30 ↗

(On Diff #36505)

Does --enable-nls work ? if so, please use NLS_CONFIGURE_ENABLE=nls

[jwb]

NLS is on by default, so I'm thinking we need to explicitly disable it for the option to have any effect:

#!/bin/sh

PACKAGE=grep "^PACKAGE = " .default | awk -F ' = ' '{print $2}'
PREFIX=grep "^PREFIX = " .default | awk -F ' = ' '{print $2}'
DATADIR=grep "^DATADIR = " .default | awk -F ' = ' '{print $2}'
ETCDIR=grep "^ETCDIR = " .default | awk -F ' = ' '{print $2}'

BSD=0
UNAME="uname"
if [ "$UNAME" = "FreeBSD" -o "$UNAME" = "OpenBSD" -o "$UNAME" = "NetBSD" ]; then

BSD=1

fi

GTK=gtk+-2.0

DEBUG=N
NLS=Y

[mat]

In D13463#281372, @jwb wrote:

NLS is on by default, so I'm thinking we need to explicitly disable it for the option to have any effect:

The only time you are allowed to use _OFF and _ON is when the more generic helpers, do not work. For example, if only --disable-nls works, but --enable-nls breaks the configure script.
See 5.13.2. Feature Auto-Activation

[mat]

Also opted not to include DOCS and EXAMPLES as options since they only install 4 files combined.

When a port uses PORTDOCS and/or PORTEXAMPLES, you MUST add DOCS and/or EXAMPLES options.

x11/lilyterm/pkg-plist

2 ↗

(On Diff #36505)

@sample

[jwb]

Think I've got it now...

Verified installation and non-installation of locale files and reran poudriere
for good measure.

[jrm]

x11/lilyterm/Makefile

28 ↗

(On Diff #36545)

For consistency, remove to keep the default NLS description.

x11/lilyterm/pkg-plist

2 ↗

(On Diff #36545)

You still need @sample etc/xdg/lilyterm.conf.sample here (check details in case I'm missremembering). See the Configuration Files section of the PH.

4 ↗

(On Diff #36545)

I'm not a desktop envirnoment user, but I recall needing USES=destkop-file-utils when a desktop entry is installed. Could you check this?

[jwb]

1. Porter's Handbook section 5.13.1.2. Syntax has this:

Tip:

When describing options, view it from the perspective of the user: “What functionality does it change?” and “Why would I want to enable this?” Do not just repeat the name. For example, describing the NLS option as “include NLS support” does not help the user, who can already see the option name but may not know what it means. Describing it as “Native Language Support via gettext utilities” is much more helpful.

2. Updating to fix config file issue...

3. Neither the porter's handbook nor the .mk file indicate *when* USES=desktop-file-utils is needed. I think the docs could use some enhancement here. If I add it, I get this:

> Running Q/A tests (stage-qa)

Warning: you may not need USES=desktop-file-utils

> Cleaning for lilyterm-0.9.9.4

x11-wm/compton provides an analogous example and it does not have USES=desktop-file-utils.

[jwb]

Fix config file issue. Awaiting feedback on other items...

[jrm]

In D13463#281681, @jwb wrote:

1. Porter's Handbook section 5.13.1.2. Syntax has this:

Tip:

When describing options, view it from the perspective of the user: “What functionality does it change?” and “Why would I want to enable this?” Do not just repeat the name. For example, describing the NLS option as “include NLS support” does not help the user, who can already see the option name but may not know what it means. Describing it as “Native Language Support via gettext utilities” is much more helpful.

Common options have default descriptions in Mk/bsd.options.desc.mk. The example you referenced is using the old description for NLS before r303137. Since the change was made under Mk/, in theory, there was a concencus that the new description is appropriate.

2. Updating to fix config file issue...

3. Neither the porter's handbook nor the .mk file indicate *when* USES=desktop-file-utils is needed. I think the docs could use some enhancement here. If I add it, I get this:

> Running Q/A tests (stage-qa)

Warning: you may not need USES=desktop-file-utils

> Cleaning for lilyterm-0.9.9.4

x11-wm/compton provides an analogous example and it does not have USES=desktop-file-utils.

Ok, I lean towards leaving it out, but it's your call. If you are motivated (crazy), you could test in gnome/kde. Worse case, the gnome/kde users won't see a pretty icon. I'm OK with that. :-P

[jwb]

Removed NLS_DESC.

I'll leave out desktop-file-utils for now, since stage-qa suggested it.

Made notes to myself to look into this and the NLS_DESC example in the
handbook.

[jwb]

Thanks for all the feedback, guys. This one is out of my usual realm as well. Most of my ports are scientific apps running on headless servers...

This is the nicest little terminal emulator I've found to date, though. I plan to use it with Lumina for lightweight desktop setups via sysutils/desktop-installer.

[mat]

head/x11/lilyterm/Makefile

25

You are using both PORTDOCS and PORTEXAMPLES, you MUST add DOCS and EXAMPLES options.

[jwb]

I removed them from the Makefile, which I thought was the issue. Are you referring to the %%PORTDOCS%% and %%PORTEXAMPLES%% in pkg-plist? If so, I can remove them as well. They were generated by makeplist.

Thanks,

JB

[jrm]

@mat means OPTIONS_DEFINE= DOCS EXAMPLES NLS, but, based on what you wrote earlier, I think you prefer to remove %%PORTDOCS%% and %%PORTEXAMPLES%% from pkg-plist. Sorry, I missed that.

[mat]

You seem to be missing the point, I will try another way.

If you install files in DOCSDIR, they MUST be guarded by PORTDOCS, and you MUST have a DOCS option.
If you install files in EXAMPLESDIR, they MUST be guarded by PORTEXAMPLES, and you MUST have an EXAMPLES option.

It does not matter if it is only one file, if you put files there, you must have the option.

[jwb]

The first time you mentioned it, I suggested removing PORTDOCS and PORTEXAMPLES from options because they only install 4 very small files. I did not see a response to that suggestion.

The handbook doesn't state that these options are required, so I was unaware of it until now.

I will put them back...

[mat]

I thought there was a warning about it, seems not :(
I will try to cook one up.

[jwb]

What's the traditional approach when arc diff --update produces this?

Exception
ERR_CLOSED: This revision has already been closed.
(Run with --trace for a full exception trace.)

This is the only change and I verified the install with both options enabled and disabled.

Index: Makefile

• Makefile (revision 456267)

+++ Makefile (working copy)
@@ -22,7 +22,7 @@
GH_ACCOUNT= Tetralet
GH_PROJECT= LilyTerm

-OPTIONS_DEFINE= NLS
+OPTIONS_DEFINE= DOCS EXAMPLES NLS
OPTIONS_SUB= yes

NLS_USES= gettext

[mat]

In D13463#281931, @jwb wrote:

What's the traditional approach when arc diff --update produces this?

Exception
ERR_CLOSED: This revision has already been closed.

As it says, this review has been closed, you should open a new one, because this is a new, even if related, patch. (Or you could click on "add action/reopen", then it'll be open, and you will be able to update.

[jwb]

One more patch needed

[jwb]

Add DOCS and EXAMPLES to OPTIONS

Retested install with options enabled and disabled

[jrm]

share/examples/lilyterm/lilyterm.conf is the same file as etc/xdg/lilyterm.conf, right? Is there some autotools foo to prevent installation of share/examples/lilyterm/lilyterm.conf? (I guess yes, since turning off EXAMPLES prevents installation). If so, then you could remove the EXAMPLES option and %%PORTEXAMPLES%%%%EXAMPLESDIR%%/lilyterm.conf from pkg-plist.

[jwb]

Yes, they are the same.

The configure script is ultra-minimal, so we'd have to manually remove EXAMPLES_DIR or patch their build system.

I did discover an undocumented --enable-strip flag that would allow replacing

${STRIP_CMD} ${STAGEDIR}${PREFIX}/bin/lilyterm

with

CONFIGURE_ARGS+= --enable-strip

I'm not sure this is an improvement.

There is no install-strip target either, so setting INSTALL_TARGET does not work.

[jrm]

I'll leave that to your discretion. I am curious how ${EXAMPLESDIR}/lilyterm.conf is prevented from being installed when the EXAMPLES option is off.

[mat]

In D13463#282085, @jwb wrote:

I did discover an undocumented --enable-strip flag that would allow replacing

${STRIP_CMD} ${STAGEDIR}${PREFIX}/bin/lilyterm

with

CONFIGURE_ARGS+= --enable-strip

I'm not sure this is an improvement.

There is no install-strip target either, so setting INSTALL_TARGET does not work.

It may not be faster to do so, as it must be disablable when building WITH_DEBUG=yes. (In which case, STRIP_CMD does not strip.)

[jwb]

It *is* staged, so the build system need not worry about it, but the ports install target works from the plist and ignores whatever is commented out.

I think I'd just remove ${STAGE_DIR}${EXAMPLES_DIR} in post-install if that's OK. Seems to be a pretty common practice, if that means anything...

[jwb]

My instinct said to stick with the canonical method (STRIP_CMD), now I know why. Thanks...

[jwb]

Remove redundant conf file from EXAMPLES_DIR

A couple more changes to default key bindings to eliminate conflicts
with common terminal operations (e.g. ctrl+q)

Update man page to match changes to default key bindings

[jrm]

This looks reasonable to me. I believe you should silence the ${RM} with @{RM} and bump PORTREVISION.

[jwb]

Yeah, feels like the same revision since I reopened the differential. Too much to keep track of for my worn-out brain...

I wasn't sure about the RM. Generally I silence things except under the install target. Looking at examples with grep, it's about half and half. Found this in the handbook, though:

5.16. Installing Files
Important:

The install phase is very important to the end user because it adds files to their system. All the additional commands run in the port Makefile's *-install targets should be echoed to the screen. Do not silence these commands with @ or .SILENT.

It clearly says "All" commands, but I could see RM being an exception. If that's the case, maybe the handbook (and portlint) should be updated to note exceptions?

[jrm]

We could wait to see if @mat responds. Based on his ports (e.g., lang/perl5-devel) I guess he will prefer silencing the RM. If this is preferred, I agree, the PH could use clarification on this point.

[jwb]

No hurry on my end, especially since we have a working port already committed. Let's use this as an opportunity to learn and maybe improve the docs before it's committed.

[jrm]

The plot thickens. @swills and @bdrewery both think the $RM should not be silenced in the *-install target. @adamw added, "Except for mkdir. We are prejudiced against mkdir commands."

[jwb]

Perhaps there could be agreement on a general principal regarding what should be echoed?

The consensus as I read it so far would be "Echo all commands that add files to the installation, but not those that create directories or remove things from stagedir".

Otherwise, this could turn into a bike shed discussion if we focus on specifics.

[jrm]

Agreed. While I think consistency, attention to detail, correctness, etc. are all important, we're into the weeds. Proceed at your discretion.

[jrm]

Oh, but since the package will change, bumping PORTREVISION is necessary.

[jwb]

I'd lean toward echoing STRIP_CMD, so I'd amend that to

"Echo all commands that add files to or alter files in the installation, but not those that create directories or remove things from stagedir"

I think the idea is to display all "useful" information about the final product.

[jwb]

Add PORTREVISION, silence RM

[jwb]

In D13463#282213, @jrm wrote:

Oh, but since the package will change, bumping PORTREVISION is necessary.

Right, I'd already fixed that yesterday. Thanks...

[jwb]

So I got an email at 8:46pm stating that the revision was accepted, but the commit message stated that it was in a needs review state. Guessing that's because I restored the svn tag line to match the previous commit and forgot to do an arc update after. My working copy contained $FreeBSD$ and I was afraid the commit might have problems if this line differed from the previous commit. Based on what I've seen tonight, I'm thinking the old content is ignored and the new tag is inserted in its place.