[WIP] add libvterm support #1130

brotzeit · 2018-12-08T18:24:50Z

Thanks to emacs-libvterm vterm seems to work in remacs.
I just pulled in the code from the module and I will now start the cleanup.
Any kind of help is appreciated, but the PR isn't ready for a review. If you want to improve something just push to this branch.

TODO:

use pseudovector instead of Lisp_Misc_Save_Value
use libvterm functions where possible
update make stuff

shaleh · 2018-12-09T03:22:59Z

configure.ac

@@ -3743,6 +3743,17 @@ fi
 AC_SUBST(LIBXML2_LIBS)
 AC_SUBST(LIBXML2_CFLAGS)

+HAVE_LIBVTERM=no
+AC_DEFINE(HAVE_LIBVTERM, 1, [do I need this])


Yes, you need this... it should says [Define if libvterm is available.]

I will update this together with the make stuff.

shaleh · 2018-12-09T03:24:10Z

lisp/vterm.el

@@ -0,0 +1,254 @@
+(require 'subr-x)


I assume this file comes from the emacs-vterm project? Should it have copyright info?

Let's wait how much code still comes from the module when we merge this.

shaleh · 2018-12-09T03:25:15Z

rust_src/src/vterm.rs

+    remacs_sys::{
+        allocate_vterm, color_to_rgb_string, get_col_offset, global_term, refresh_lines,
+        rgb_string_to_color, row_to_linenr, term_process_key, vterm_module_copy_string_contents,
+        vterm_output_read,  vterm_sb_buffer_size, vterm_screen_callbacks,


There are also vterm items in here...

shaleh · 2018-12-09T03:28:51Z

rust_src/src/vterm.rs

+        let line_start = row_to_linenr(term, (*term).invalid_start);
+
+        call!(
+            LispObject::from(intern("goto-line")),


goto-line is interactive and so calling it programmatically is usually frowned on. Do you need all of the extra behavior of goto-line or is forward-line sufficient?

I checked it and it seems goto-line has some extra logic that is needed in one function call. I will try to find a solution without calling lisp functions.

Calling lisp is ok. Calling interactive lisp is frowned on.

shaleh · 2018-12-09T03:30:09Z

rust_src/src/vterm.rs

+
+    let cursor_lnum = row_to_linenr(term, pos.row);
+    call!(
+        LispObject::from(intern("goto-line")),


Same question about goto-line here.

shaleh · 2018-12-09T03:30:31Z

rust_src/src/vterm.rs

+
+        let buf_index = buffer_lnum - height + 1;
+        call!(
+            LispObject::from(intern("goto-line")),


shaleh · 2018-12-09T03:32:13Z

rust_src/src/vterm.rs

+}
+
+#[lisp_fn]
+pub fn vterm_write_input(input: LispObject) -> LispObject {


Suggested change

pub fn vterm_write_input(input: LispObject) -> LispObject {

pub fn vterm_write_input(input: LispStringRef) -> LispObject {

shaleh · 2018-12-09T03:32:48Z

rust_src/src/vterm.rs

+        vterm_screen_flush_damage((*term).vts);
+    }
+
+    LispObject::from(0)


Why not just return int or EmacsInt?

leonerd · 2018-12-09T13:38:03Z

src/vterm.c

+}
+
+VTermScreenCallbacks vterm_screen_callbacks = {
+    .damage = vterm_damage,


Curious. Is this attempting to make a set of callbacks that reuse the original libvterm functions in most of the slots, but just adds some scrollback handling? This feels a slightly awkward way to go about it - perhaps we can find a better way around that?

I just copied the code from emacs-libvterm and modified it so it works in remacs and didn't really take a look at libvterm, so far.

perhaps we can find a better way around that

Sounds good =)
I don't know much, but libvterm seems to work really well. Thanks for your work!

leonerd · 2018-12-09T13:39:01Z

src/vterm.c

+};
+
+void
+vterm_sb_buffer_size () {


I'd suggest not making new non-static functions in the vterm_... namespace; in case libvterm itself starts adding more.

True. I wanted to avoid confusion regarding term and vterm, but I will change it to something different.

leonerd · 2018-12-09T13:40:50Z

src/vterm.c

+    vterm_keyboard_key(term->vt, VTERM_KEY_END, modifier);
+  } else if (is_key(key, len, "<prior>")) {
+    vterm_keyboard_key(term->vt, VTERM_KEY_PAGEUP, modifier);
+  } else if (is_key(key, len, "<f0>")) {


Could maybe these be done parametrically by looking for prefix <f and suffix > then parsing the middle as being a decimal integer? That would extend beyond 12 then if required.

Makes sense.

leonerd · 2018-12-09T13:42:29Z

src/vterm.c

+}
+
+static size_t
+codepoint_to_utf8_c(const uint32_t codepoint, unsigned char buffer[4]) {


This (and the subsequent utf8_to_codepoint) are very similar to functions that already exist in libvterm. It might be easier just to expose those publicly then you can use them directly.

Good to know. I will make a TODO list.

leonerd · 2018-12-09T13:44:14Z

src/vterm.c

+  return offset;
+}
+
+static bool compare_cells(VTermScreenCell *a, VTermScreenCell *b) {


This too is almost identical to an internal libvterm function - might be best to expose and use that.

brotzeit · 2018-12-09T17:04:12Z

I tried to allocate the struct as a pseudovector similiar to terminal.

Term *
allocate_vterm (void) {
  global_term =  ALLOCATE_ZEROED_PSEUDOVECTOR
    (struct Term, next_vterm, PVEC_VTERM);
}

I added the field

struct Term *next_vterm;

to the term struct.

It compiles and runs, but after a few insertions remacs segfaults. Any idea ?

shaleh · 2018-12-09T18:21:54Z

src/lisp.h

+  bool visible;
+} VtermCursor;
+
+typedef struct Term {


Perhaps you should name this vterminal or terminal_vterm. Plain 'Term' can be confusing as long as the other terminal options are present.

shaleh · 2018-12-09T18:29:03Z

Look here @brotzeit https://github.com/Wilfred/remacs/blob/9517583be200869e076dcf4682eea4943d9a8b76/src/alloc.c#L3246 for following how the vectorlike allocations work.

Are you using malloc directly or are you using lisp_malloc?

brotzeit · 2018-12-09T19:41:03Z

Are you using malloc directly or are you using lisp_malloc?

I thought the macro is doing the allocation for pseudovectors.

shaleh · 2018-12-10T06:22:36Z

I meant for the allocation of next.

brotzeit · 2018-12-12T14:19:08Z

@shaleh I removed next for now, but it still doesn't work.

struct Term *
allocate_vterm (void) {
  return ALLOCATE_ZEROED_PSEUDOVECTOR
    (struct Term, vt, PVEC_VTERM);
}

struct Term {
  union vectorlike_header header;

  Lisp_Object dummy;

  VTerm *vt;
  VTermScreen *vts;
  // buffer used to:
  //  - convert VTermScreen cell arrays into utf8 strings
  //  - receive data from libvterm as a result of key presses.
  VtermScrollbackLine **sb_buffer; // Scrollback buffer storage for libvterm  
  VtermCursor cursor;
  
  bool is_invalidated;
  
  size_t sb_current;          // number of rows pushed to sb_buffer
  size_t sb_size;             // sb_buffer size
  // "virtual index" that points to the first sb_buffer row that we need to
  // push to the terminal buffer when refreshing the scrollback. When negative,
  // it actually points to entries that are no longer in sb_buffer (because the
  // window height has increased) and must be deleted from the terminal buffer
  int sb_pending;

  int invalid_start, invalid_end; // invalid rows in libvterm screen
};

I've read the docs a little more and when I understand correctly non LispObjects have to be after the last LispObject and you tell ALLOCATE_ZEROED_PSEUDOVECTOR with the second argument where it has to start allocating different types.

brotzeit · 2018-12-13T10:06:01Z

@db48x @Wilfred any idea ?

brotzeit · 2018-12-27T10:45:50Z

It seems bindgen has problems handling the libvterm enum VTermProp.

error[E0573]: expected type, found module `VTermProp`
   --> src/vterm.rs:537:11
    |
537 |     prop: VTermProp,
    |           ^^^^^^^^^ did you mean `VTermPos`?

I've never seen this before. Any idea ?

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

#1212

brotzeit force-pushed the vterm branch from de06238 to 518684b Compare December 8, 2018 23:07

shaleh reviewed Dec 9, 2018

View reviewed changes

rust_src/src/vterm.rs Outdated

let buf_index = buffer_lnum - height + 1;

call!(

LispObject::from(intern("goto-line")),

Copy link

Collaborator

shaleh Dec 9, 2018

Choose a reason for hiding this comment

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

and here

shaleh reviewed Dec 9, 2018

View reviewed changes

leonerd reviewed Dec 9, 2018

View reviewed changes

shaleh reviewed Dec 9, 2018

View reviewed changes

brotzeit force-pushed the vterm branch from 5c574db to 466b9d5 Compare December 10, 2018 20:17

brotzeit force-pushed the vterm branch from dcb7203 to 29f0834 Compare December 13, 2018 14:35

brotzeit mentioned this pull request Dec 14, 2018

Help needed: libvterm integration for remacs akermu/emacs-libvterm#49

Open

brotzeit force-pushed the vterm branch 3 times, most recently from ac02ba4 to d27082c Compare December 21, 2018 20:39

brotzeit force-pushed the vterm branch 2 times, most recently from f4c90a6 to 72bbb6e Compare December 26, 2018 19:18

brotzeit force-pushed the vterm branch from 68416e2 to a835e5f Compare January 2, 2019 20:17

brotzeit added 4 commits January 3, 2019 01:49

add libvterm

61efb78

use fetch_char

e5f3a06

add is_eol

6d64227

remove vterm_sb_buffer_size

790b6c7

brotzeit force-pushed the vterm branch from a835e5f to 19ae959 Compare January 3, 2019 15:10

brotzeit added 3 commits January 3, 2019 17:19

change cursor visibility

19ae959

port term_settermprop

b74c01b

term_process_key

fcbfed5

brotzeit closed this Jan 5, 2019

brotzeit mentioned this pull request Jan 8, 2019

add libvterm support #1212

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] add libvterm support #1130

[WIP] add libvterm support #1130

brotzeit commented Dec 8, 2018 •

edited

shaleh Dec 9, 2018

brotzeit Dec 9, 2018

shaleh Dec 9, 2018

brotzeit Dec 9, 2018

shaleh Dec 9, 2018

shaleh Dec 9, 2018

brotzeit Dec 9, 2018

shaleh Dec 9, 2018

shaleh Dec 9, 2018

shaleh Dec 9, 2018

shaleh Dec 9, 2018

shaleh Dec 9, 2018

leonerd Dec 9, 2018

brotzeit Dec 9, 2018

leonerd Dec 9, 2018

brotzeit Dec 9, 2018

leonerd Dec 9, 2018

brotzeit Dec 9, 2018

leonerd Dec 9, 2018

brotzeit Dec 9, 2018

leonerd Dec 9, 2018

brotzeit commented Dec 9, 2018

shaleh Dec 9, 2018

shaleh commented Dec 9, 2018 •

edited

brotzeit commented Dec 9, 2018

shaleh commented Dec 10, 2018

brotzeit commented Dec 12, 2018 •

edited

brotzeit commented Dec 13, 2018

brotzeit commented Dec 27, 2018

brotzeit commented Jan 5, 2019

	pub fn vterm_write_input(input: LispObject) -> LispObject {
	pub fn vterm_write_input(input: LispStringRef) -> LispObject {

[WIP] add libvterm support #1130

[WIP] add libvterm support #1130

Conversation

brotzeit commented Dec 8, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzeit commented Dec 9, 2018

Choose a reason for hiding this comment

shaleh commented Dec 9, 2018 • edited

brotzeit commented Dec 9, 2018

shaleh commented Dec 10, 2018

brotzeit commented Dec 12, 2018 • edited

brotzeit commented Dec 13, 2018

brotzeit commented Dec 27, 2018

brotzeit commented Jan 5, 2019

brotzeit commented Dec 8, 2018 •

edited

shaleh commented Dec 9, 2018 •

edited

brotzeit commented Dec 12, 2018 •

edited