GNOME Bugzilla – Bug 728900
use terminfo instead of termcap
Last modified: 2014-05-20 09:26:47 UTC
On wip/terminfo branch. The keyboard sequences emitted aren't quite right yet, so I backed it out for now until I can check that.
Bug 600659 comment 68 mentions some cleanup possibilities that could be addressed here.
Any chance we could (optionally) get rid of libncurses (libtinfo) dependency? I received such a request from Debian vte maintainer to upstream their 25_optional-ncurses.patch if feasible (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=747347#30).
With the branch the key sequences emitted are all wrong (or at least different from before); until I/someone investigates that, this branch can't land. I'd love to not depend on libtinfo, but that would mean writing our own parser for the terminfo compiled format, so it's not high priority for me.
I think it's more of a higher level question. VTE used to be the "emulate whatever is in the terminfo/termcap" emulator, but now it's a mixture of this and plenty of hardcoded sequences. Is there any point keeping this, and keeping the ability of emulating anything other than xterm? (There's another bug that non-xterm emulation has been buggy for a long time.) Or can/should we just totally get rid of this approach and use hardcoded xterm sequences? I agree there's not much point using our own solution if there's a library for exactly what we need.
In current master (old termcap version), keymap.c _vte_keymap_map(), inside the "Search for the conditions" loop, the "else" branch never seems to find a termcap entry for me (not sure why), _vte_termcap_find_string() always returns the empty string and hence it skips it. But it stores the termcap name in termcap_special, and after the loop it is correctly looked up. This means that the first matching hardwired ("normal") sequence is used, but if there's no hardwired sequence matching then the last(!) matching termcap ("special") entry is used instead. Hardly makes any sense to me. You've apparently modified it in the terminfo version to a single scan from the beginning to the end, using the first nonempty value, regardless of whether it's a hardwired string or a successful terminfo lookup. Obviously this is the behavior one would expect by looking at the _vte_keymap_* definitions. This seems to be the reason for wrong/different escape sequences. E.g. in case of Home it now takes terminfo's khome as per the first rule, which is ^[OF, whereas previously the kh termcap lookup failed and the hardwired ^[[F sequence was used from a subsequent rule. Note: See bug 600659 comment 74 for an automated way of testing the sequences. Do we want to rely in term{cap,info} *at all*? So far vte has used hardwired sequences most of the time. Even if we clean up everything and drop all fkey modes except the default (request: bug 600659 comment 73) we'd need to keep hardwired sequences for numerous reasons. E.g. Home/End should generate ^[[H/^[[F which are not present in terminfo. Application cursor keys and application keypad mode alter some sequences, with AFAICT no terminfo support whatsoever. F1..F4 completely change their sequences when a modifier is pressed, again probably no support for it in terminfo. Terminfo is just able to encode the complexity (app keypad mode, app cursor mode, modifiers, numlock) we need. Wouldn't life be much simpler with just hardwired xterm-compatible sequences? (With probably a way to override them from config file or dconf for experts.)
Typo: Terminfo is just *unable* to encode the complexity we need.
(In reply to comment #5) > Do we want to rely in term{cap,info} *at all*? So far vte has used hardwired > sequences most of the time. Even if we clean up everything and drop all fkey > modes except the default (request: bug 600659 comment 73) we'd need to keep > hardwired sequences for numerous reasons. E.g. Home/End should generate > ^[[H/^[[F which are not present in terminfo. Application cursor keys and > application keypad mode alter some sequences, with AFAICT no terminfo support > whatsoever. F1..F4 completely change their sequences when a modifier is > pressed, again probably no support for it in terminfo. Terminfo is just able to > encode the complexity (app keypad mode, app cursor mode, modifiers, numlock) we > need. Wouldn't life be much simpler with just hardwired xterm-compatible > sequences? (With probably a way to override them from config file or dconf for > experts.) Makes sense to me.
Agreed, we should just hardcode xterm behaviour. No need to make this dconf/config file configurable; but I would suggest to use #define (or static strings) defined in one file instead of sprinkling the literal strings throughout the code. Since wip/terminfo still is a nice cleanup, should start from there (unless there's a compelling reason against, of course).
Do you agree with dropping fkey modes (bug 730137)? I really don't feel like fixing them, and porting them to terminfo or hardwired codes is a huge burden. If so, can I do that and drop terminfo in the wip/terminfo (or in a different?) branch?
I've pushed the branch to master. Since comment 5 found the problem with the sequence lookup, and since this code is going to go away, I just moved the lookup outside the loop as last resort. Thanks for finding the problem!
I'm done with the tough part of getting rid of terminfo. vteseq contained lots of duplicated stuff, most of which is removed now. Hopefully I didn't mess up anything. vteseq-2 is now empty. caps.c and vteseq-n contain the hardwired definition of non-key input escape sequences. TODO: - Actually remove vteseq-2 and any terminfo/ncurses dependency, cleanup any related code as much as possible. - Remove trie matcher (bug 728665) - Further easy cleanups in vteseq.c (e.g. rearrange the methods in some sane order). - Figure out cleanup_tab_fragments (bug 730343) - Maybe remove normal_length in keymap (instead: either a runtime strlen or a compile-time sizeof magic uniformly for all the strings) - anything else?
Created attachment 276792 [details] [review] Completely drop terminfo/ncurses dependency Here's a patch that drops ncurses completely. ChPe, could you please take a look? It's a bit big, but it wouldn't have made too much sense to stop anywhere in the middle. The only remaining task of vte_terminal_set_emulation() would have been to set $TERM for the children, so the name would have been quite misleading, I should have at least renamed to vte_terminal_set_term(). And if I need to touch it anyways, keeping the infrastructure solely for carrying this value (which is no longer used by VTE itself) wouldn't make too much sense, the envv parameter of vte_terminal_spawn_sync() can do the same. So I believe this is the right thing to do. I removed all these methods. If someone wishes to fork a child with a non-default TERM, they need to add it in env. A fallback default ("xterm") is automatically added so we make vte app developers' life easier. There's a new fixme in pty.c, I believe that code is useless there but I'm waiting if you have more info.
Created attachment 276800 [details] [review] Completely drop terminfo/ncurses dependency, v2 v2: I realized dropping GCache was a bad idea, because then we have a separate (but identical) matcher for all terminals. Since GCache is an overkill for this and deprecated, I solved it "manually" by a singleton matcher.
+ /* Make sure the one in envp overrides the default. */ + g_hash_table_replace (table, g_strdup ("TERM"), g_strdup (VTE_DEFAULT_TERM)); Hmm. Shouldn't this lookup TERM first and only if it's not present override it? Or maybe we should just remove TERM handling altogether and leave it to the calling app ? Other than this bit, looks great; thanks! (In reply to comment #11) > - Maybe remove normal_length in keymap (instead: either a runtime strlen or a > compile-time sizeof magic uniformly for all the strings) Yes, that bit of code is really stupid. > - anything else? Longer-term, maybe this: now that we don't load sequences anymore but hardcode them, our flexible matcher setup (table.c remaining with trie.c gone) is rather overkill. Let's look into replacing it with a simpler mechanism, maybe take inspiration from libtsm or terminology.
Why is this marked Fixed? I don't see the latest patch landed yet. Great job both of you!
> Hmm. Shouldn't this lookup TERM first and only if it's not present override it? I think it's better to ignore the host app's TERM. E.g. you run urxvt and there type vte, you'd end up with TERM=urxvt. If a host app really wishes to transparently pass through its TERM (for which I see no reason), it can duplicate in envv. > Or maybe we should just remove TERM handling altogether and leave it to the > calling app ? This would again have the same problem with lazy vteapp-clone coders. I think it's an advantage if it takes one less method call to get a working terminal. This is just a simple safeguard so that if you write a vte-app clone with the least effort, it already works correctly.
Review of attachment 276800 [details] [review]: Still not quite sure about the TERM handling, but I'll conduct some experiments after this patch lands. Please commit, thanks!