add libvterm support #1212

brotzeit · 2019-01-05T12:25:32Z

I have a TODO list of the review from the old PR, but I would like to get this merged as I still hope I get some help with this =)

However I still have problems with configure.ac. I can't get this to work.
.rustified_enum("VTermProp") doesn't work since #1185. It worked before...

Apart from that, it seems to be usable. But resizing needs to get fixed.

shaleh · 2019-01-06T03:01:34Z

Perhaps the right thing to do is leave this in a feature branch while the kinks are worked through.

brotzeit · 2019-01-06T19:09:14Z

@shaleh ok I will work some more on this. Can you please take a look at configure.ac. I don't know why HAVE_LIBVTERM doesn't work.

shaleh · 2019-01-06T23:44:10Z

@brotzeit I pushed a fix to the configure script and used it to make vterm.c become empty if libvterm is not found. That may need some tweaking, but the detection is now solid.

EMACS_CHECK_MODULES is all you needed. It uses pkgconfig to detect the library in question. An extra gotcha is the library defines itself as vterm in its .pc file, not libvterm.

When EMACS_CHECK_MODULES finds a library it sets HAVE_$LIBRARY=yes for you. Otherwise it is set to no. The extra bit which looks weird is you then have to call AC_DEFINE which exports the HAVE_FOO so it is in config.h and usable in C land.

shaleh · 2019-01-07T00:14:35Z

Ok, I just pushed some changes that should enable you to allocate the vterminal as a pseudovector. vterm_new is not defined so I could not easily test it. Details are in the commit message.

brotzeit · 2019-01-08T10:51:25Z

@shaleh Thanks. Your changes regarding the pseudovector were the same as mine, but it didn't work when I tried this because we didn't tag.

With LispObject::tag_ptr(v, Lisp_Type::Lisp_Vectorlike) it seems to work fine.

shaleh · 2019-01-08T21:15:43Z

@brotzeit you also missed adding the header to the top of struct vterminal.

brotzeit · 2019-01-08T21:21:20Z

No, I had that one. First post after your last post in #1130 ;)

I guess without that it wouldn't have run in the first place. It was just the segfault after a few insertions when vterm was already running.

Tagging solved the problem. I wonder if

    pub fn from_ptr(ptr: *mut c_void) -> Option<Self> {
        unsafe { ptr.as_ref().map(|p| mem::transmute(p)) }
    }

is also a source of potential problems ?

shaleh · 2019-01-08T21:44:07Z

Yeah, can be potentially dangerous if the wrong type is used. However, it is taking a pointer after it has been untagged. And the object returned should be retagged. If that is true it is safe.

shaleh · 2019-01-10T21:11:40Z

rust_src/build.rs

 fn ignore(path: &str) -> bool {
    path == "" || path.starts_with('.') || path == "lib.rs" || path == "functions.rs"
 }

+#[cfg(not(feature = "libvterm"))]


Instead of repeating the base ignore maybe you should make a base_ignore which is all that the vterm version uses and the non-vterm version is it plus vterm.rs.

shaleh · 2019-01-10T21:12:33Z

rust_src/src/vterm.rs

@@ -53,9 +53,9 @@ impl LispVterminalRef {
        (height, width)
    }

-    pub fn set_size(self, rows: EmacsInt, cols: EmacsInt) {
+    pub fn set_size(self, rows: c_int, cols: c_int) {


Do we want to continue using c_int or should we transition to i32?

brotzeit · 2019-01-12T23:34:41Z

I have another problem that confuses me. When I use describe-variable with the buffer local variable vterm--term in vterm-mode, I get a segfault.

brotzeit · 2019-01-12T23:40:02Z

And resizing horizontally after window size change also doesn't work correctly.

brotzeit · 2019-01-14T11:42:04Z

vterm_set_size only sets the height and not the width. @leonerd any idea what I'm doing wrong here ? It seems to work for neovim...

leonerd · 2019-01-14T12:23:30Z

Hrm.. werird. That should set both height and width at the same time. Do you have a link to the relevant code? I can't see a call to vterm_set_size in the changes in this PR

brotzeit · 2019-01-14T12:53:58Z

rust_src/src/vterm.rs

+
+    pub fn set_size(self, rows: i32, cols: i32) {
+        unsafe {
+            vterm_set_size((*self).vt, rows, cols);


@leonerd Here. When I resize the window while htop is running it only updates the height. But when I exit htop and run it again it also gets the correct width. Maybe the reason is how emacs handles resizing...

The issue is caused by the COLUMNS and LINES environment variable. Unset these before invoking htop and resizing will work.

brotzeit · 2019-02-05T09:24:44Z

I wonder if it wouldn't be better to vendor libvterm. neovim and emacs-libvterm both do it this way and it seems this is the reason why libvterm isn't up to date in the package repos. The arch linux package is orphaned. I could take care of the arch linux package, but what's with the other distros.

What do you think ?

leonerd · 2019-02-05T14:44:31Z

I don't know quite what "vendoring" means, but generally at the moment you want to be taking yourself a static snapshot of whatever version you've written code against, and only update that when you're updating code at the same time. Until I get around to actually stablising any last bits of API I know I'm going to be changing in future, I am not making version-numbered releases of libvterm. Best is to keep that static snapshot so you know APIs aren't going to change underneath you.

brotzeit · 2019-02-05T14:51:35Z

@leonerd I have read the term in neovim and emacs-libvterm so I thought this describes what we both mean ;)

But that's what I thought. Thanks.

shaleh · 2019-02-05T16:04:08Z

@brotzeit once @Wilfred sets up the remacs project you could bring libvterm there instead of adding more code to this quite large repo.

brotzeit · 2019-02-06T11:46:43Z

@shaleh Can you give me some source on how to add this to the build process. I was hoping I can take this from emacs-libvterm but I guess I can't use this cmake stuff ?

shaleh · 2019-02-14T22:52:12Z

My expectation is the user first builds the vterm library and puts it somewhere linkable. For instance in /usr/local/lib. Then they build remacs and the remacs configure script will find and set up the linking for vterm. Does this match your expectation @brotzeit?

This is how I am accustomed to handling C/C++ libraries in other open source projects. Look at the configure script for remacs. It fails and says "go figure libjpeg out and come back". Welcome to the days before bundling. The user was expected to either use a tool like apt/yum/whatever or to download and compile the package. Once done you go back and try again. Repeat until all dependencies are installed or you give up and use something already packaged.

brotzeit · 2019-02-16T09:59:46Z

The only problem I see is that users get the wrong version for our remacs implementation. That's the reason why you told me to add libvterm to the remacs organization right ?

shaleh · 2019-02-17T02:15:40Z

Having the crate in the remacs org makes keeping the versions in sync easier. The user installs the vterm lib and then builds remacs.

brotzeit · 2019-03-10T16:28:16Z

I gave up with libvterm as a submodule. It seemed to me like a nice idea, but I don't get how this works.

I think with @akermu 's fixes it works pretty well. It would be great if we could start the final review.

brotzeit · 2019-03-13T13:59:10Z

Scrollback still needs fixing, but since we make this optional this should be ok. We just mention in the readme libvterm support is not stable. Maybe some users get motivated to help with it when they see it works.

leonerd · 2019-08-28T10:28:34Z

It might be of interest to you folks that I've now made a v0.1 tarball release. Though bits of API are still verymuch subject to change, so expect that v0.2 will probably have some incompatible bits in it.

http://www.leonerd.org.uk/code/libvterm/

brotzeit mentioned this pull request Jan 5, 2019

[WIP] add libvterm support #1130

Closed

3 tasks

brotzeit requested a review from shaleh January 5, 2019 12:41

shaleh reviewed Jan 10, 2019

View reviewed changes

brotzeit force-pushed the vterm_new branch from 31521ec to 410d212 Compare January 13, 2019 19:36

brotzeit commented Jan 14, 2019

View reviewed changes

brotzeit force-pushed the vterm_new branch 2 times, most recently from 0b08cb7 to 35a228a Compare January 17, 2019 20:52

add libvterm

35a228a

brotzeit mentioned this pull request Feb 3, 2019

libvterm terminal emulation #968

Open

Fix htop window resizing

7b63a5a

akermu and others added 2 commits February 19, 2019 23:18

Fix multiple vterm sessions

2144b51

update stuff

5deb998

add libvterm installation guide

72e7463

brotzeit force-pushed the vterm_new branch 2 times, most recently from 7ba4871 to f72b63e Compare March 10, 2019 17:13

Merge branch 'master' into vterm_new

d53112a

brotzeit force-pushed the vterm_new branch 2 times, most recently from c622d83 to 852a27a Compare March 10, 2019 18:51

brotzeit force-pushed the vterm_new branch from c1fa03b to 7110c47 Compare March 30, 2019 09:52

brotzeit mentioned this pull request Apr 15, 2019

Port window-pixel-width-before-size-change #1429

Closed

brotzeit force-pushed the vterm_new branch 3 times, most recently from 7110c47 to 9a60b44 Compare April 19, 2019 13:42

brotzeit force-pushed the vterm_new branch from 9a60b44 to 8c028dd Compare May 11, 2019 08:37

brotzeit added 9 commits May 11, 2019 12:50

update stuff

8c028dd

vterminal_goto_line

2521af4

remove goto-line

e8062cc

type stuff

55abba9

utf8

9eb4d1f

back to emacsint

813387d

add missing is_eol in get_col_offset

089ac5c

more stuff

331eb8f

port get_col_offset

084fa0a

brotzeit mentioned this pull request May 12, 2019

port allocate_zeroed_pseudovector #1492

Closed

brotzeit closed this Aug 28, 2019

brotzeit deleted the vterm_new branch August 28, 2019 10:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add libvterm support #1212

add libvterm support #1212

brotzeit commented Jan 5, 2019 •

edited

shaleh commented Jan 6, 2019

brotzeit commented Jan 6, 2019

shaleh commented Jan 6, 2019

shaleh commented Jan 7, 2019

brotzeit commented Jan 8, 2019 •

edited

shaleh commented Jan 8, 2019

brotzeit commented Jan 8, 2019 •

edited

shaleh commented Jan 8, 2019

shaleh Jan 10, 2019

shaleh Jan 10, 2019

brotzeit commented Jan 12, 2019

brotzeit commented Jan 12, 2019

brotzeit commented Jan 14, 2019

leonerd commented Jan 14, 2019

brotzeit Jan 14, 2019

akermu Feb 14, 2019

brotzeit commented Feb 5, 2019

leonerd commented Feb 5, 2019

brotzeit commented Feb 5, 2019

shaleh commented Feb 5, 2019

brotzeit commented Feb 6, 2019

shaleh commented Feb 14, 2019 •

edited

brotzeit commented Feb 16, 2019

shaleh commented Feb 17, 2019

brotzeit commented Mar 10, 2019 •

edited

brotzeit commented Mar 13, 2019

leonerd commented Aug 28, 2019

add libvterm support #1212

add libvterm support #1212

Conversation

brotzeit commented Jan 5, 2019 • edited

shaleh commented Jan 6, 2019

brotzeit commented Jan 6, 2019

shaleh commented Jan 6, 2019

shaleh commented Jan 7, 2019

brotzeit commented Jan 8, 2019 • edited

shaleh commented Jan 8, 2019

brotzeit commented Jan 8, 2019 • edited

shaleh commented Jan 8, 2019

shaleh Jan 10, 2019

Choose a reason for hiding this comment

shaleh Jan 10, 2019

Choose a reason for hiding this comment

brotzeit commented Jan 12, 2019

brotzeit commented Jan 12, 2019

brotzeit commented Jan 14, 2019

leonerd commented Jan 14, 2019

brotzeit Jan 14, 2019

Choose a reason for hiding this comment

akermu Feb 14, 2019

Choose a reason for hiding this comment

brotzeit commented Feb 5, 2019

leonerd commented Feb 5, 2019

brotzeit commented Feb 5, 2019

shaleh commented Feb 5, 2019

brotzeit commented Feb 6, 2019

shaleh commented Feb 14, 2019 • edited

brotzeit commented Feb 16, 2019

shaleh commented Feb 17, 2019

brotzeit commented Mar 10, 2019 • edited

brotzeit commented Mar 13, 2019

leonerd commented Aug 28, 2019

brotzeit commented Jan 5, 2019 •

edited

brotzeit commented Jan 8, 2019 •

edited

brotzeit commented Jan 8, 2019 •

edited

shaleh commented Feb 14, 2019 •

edited

brotzeit commented Mar 10, 2019 •

edited