vis icon indicating copy to clipboard operation
vis copied to clipboard

Soft word wrapping

Open proskur1n opened this issue 3 years ago • 3 comments

Hello, this pull request adds soft word wrapping (linebreak in vim) support to vis. In addition, it defines an option to change the visual line length instead of always using window width for the maximum line length. The latter was made trivial by my code rewrite.

Rationale

It is a lot of pain to write LaTeX and Markdown documents without proper word wrapping support in the editor. This option is something that makes your life much easier and is supported by nearly all other text editors. In fact, a similar functionality was requested by #142, but on the contrary to that proposal, this pull request only affects the way lines are displayed on screen and doesn't modify the underlined character data.

New options

  • breakat / brk specifies a list of characters after which the editor may decide to wrap the line before window width is reached. The default value for this option is an empty string which corresponds to the old behavior of vis. In this case, cells are just printed one after another until window width is reached.
  • wrapcolumn / wc affects the maximum displayed line length and is especially useful for large monitors. Zero is the default for this option and thus disables this feature. I didn't exactly know how to name this option, but "wrapcolumn" sounds pretty good to me.

I have included a little screenshot with both of the new options enabled below.

:set wrapcolumn 80
:set breakat ' .!?'

vis_word_wrap

Todo

  • I have tried this feature with a couple of different text files and couldn't see any problems with my implementation. However, feel free to test this pull request on some strange edge cases, which may cause something to break. Anyway, my intention was to not change the default look of vis, but I had to rewrite the view_addch function in view.c, which may have caused some unintentional side effects.
  • ~~Right now, breakat only supports ASCII characters, but it shouldn't be too difficult to extend my code to handle any UTF-8 characters. In fact, vim implementation of breakat also only supports ASCII.~~

proskur1n avatar May 09 '21 21:05 proskur1n

Works for me, just it could be emphasized somewhere, that settings should go to vis.events.WIN_OPEN handler, not (more usual, I guess) vis.events.INIT one.

mcepl avatar Aug 22 '21 17:08 mcepl

Works great but I found one edge case:

image

The underline styling carries through the wrap-gap. My guess is this should be easy to clear/restore while rendering the gaps, but it's not a big deal either way (I don't think your PR introduced this behaviour, but is relevant).

7v0lk0v avatar Jun 01 '22 09:06 7v0lk0v

Anything stopping this from being merged? Seem like a good candidate

erf avatar Sep 03 '22 22:09 erf

I pulled this down locally and it works for me. This would be a nice addition. :+1:

@proskur1n I'm curious if this technique could also be used to mimic nowrap. I often open structured data files (commonly JSON but many others as well) where line lengths are thousands of characters long but I only care to see the beginning of each line/record. (I don't need horizontal scrolling, I just want to toggle wrapping so I can see individual line numbers without the noise.)

I was hoping this PR might let me set wrapcolumn to a value larger than $COLUMNS as a hack for my use-case. That doesn't work which makes complete sense -- it's a weird idea to wrap text off-screen after all. That said, would it be possible to use the same technique in this PR to disable wrapping altogether and introduce a nowrap setting?

whiteinge avatar Sep 23 '22 05:09 whiteinge

@erf I marked this pull request as a draft because there were some segfaults when scrolling through binary files. Those should be fixed by aa54c376f3b9f36700a0e74d86fa871497c5e698 now. However, it may also have introduced other bugs :)

@whiteinge I don't think this is possible at the moment. As far as I know, if you simply skip some characters when rendering, the navigation will stop working properly. It's actually a miracle that soft wrapping works at all with so few changes to the code.

On a side note, I was thinking about dropping support for breakat and simply wrapping on isspace(...) instead (With a boolean :set option to enable it). Are there any cases when you would want to wrap the lines on something else instead?

proskur1n avatar Sep 26 '22 18:09 proskur1n

Should we rename breakat to wrapat ? So that they are displayed next to each other in :help.

proskur1n avatar Jul 27 '23 21:07 proskur1n

Should we rename breakat to wrapat ? So that they are displayed next to each other in :help.

No I think breakat makes more sense and is consistent with other editors.

rnpnr avatar Jul 27 '23 23:07 rnpnr

The two commits I just pushed are what I will merge shortly. Below is the diff between what I pushed and what was here before (for posterity):

diff --git a/view.c b/view.c
index ebdb876..0b7ce36 100644
--- a/view.c
+++ b/view.c
@@ -178,9 +178,9 @@ static int view_max_text_width(const View *view) {
 }
 
 static void view_wrap_line(View *view) {
+	Line *wrapped_line = view->line;
 	int col = view->col;
 	int wrapcol = (view->wrapcol > 0) ? view->wrapcol : view->col;
-	Line *wrapped_line = view->line;
 
 	view->line = view->line->next;
 	view->col = 0;
@@ -197,7 +197,7 @@ static void view_wrap_line(View *view) {
 		}
 	}
 
-	/* clear remaining of line */
+	/* clear remaining cells on line */
 	for (int i = wrapcol; i < view->width; ++i) {
 		if (i < col) {
 			wrapped_line->width -= wrapped_line->cells[i].width;
@@ -208,9 +208,11 @@ static void view_wrap_line(View *view) {
 }
 
 static bool view_add_cell(View *view, const Cell *cell) {
-	/* at most one iteration most of the time. we have to use a while-loop
-	 * here because of some pathological edge cases where one unicode char
-	 * may be bigger than the (extremely small) terminal width. */
+	/* if the terminal is resized to a single (ASCII) char an out
+	 * of bounds write could be performed for a wide char. this can
+	 * be caught by iterating through the lines with view_wrap_line()
+	 * until no lines remain. usually 0 or 1 iterations.
+	 */
 	while (view->col + cell->width > view_max_text_width(view)) {
 		view_wrap_line(view);
 		if (!view->line)
@@ -220,11 +222,9 @@ static bool view_add_cell(View *view, const Cell *cell) {
 	view->line->width += cell->width;
 	view->line->len += cell->len;
 	view->line->cells[view->col++] = *cell;
-	for (int i = 1; i < cell->width; ++i) {
-		/* set cells of a character which uses multiple columns */
+	/* set cells of a character which uses multiple columns */
+	for (int i = 1; i < cell->width; i++)
 		view->line->cells[view->col++] = cell_unused;
-	}
-
 	return true;
 }

rnpnr avatar Jul 28 '23 18:07 rnpnr