From 81411e6b54a6c0ce31f800e3fcf178a358062f3a Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Fri, 4 Jun 2021 18:13:36 +0530 Subject: [PATCH] Fix trailing parentheses in URLs not being detected Also fix URLs starting near the end of the line not being detected. Fixes #3688 --- docs/changelog.rst | 2 + kitty/line.c | 5 +-- kitty/mouse.c | 83 +++------------------------------------- kitty/screen.c | 82 +++++++++++++++++++++++++++++++++++++++ kitty/screen.h | 1 + kitty/unicode-data.h | 5 +-- kitty_tests/datatypes.py | 7 +++- kitty_tests/screen.py | 25 ++++++++++++ 8 files changed, 124 insertions(+), 86 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1db5cad6f..32061d4a2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -71,6 +71,8 @@ To update |kitty|, :doc:`follow the instructions `. executing any command specified on the command line via the users' shell just as ssh does (:iss:`3638`) +- Fix trailing parentheses in URLs not being detected (:iss:`3688`) + - Tab bar: Use a lower contrast color for tab separators (:pull:`3666`) - Fix a regression that caused using the ``title`` command in session files diff --git a/kitty/line.c b/kitty/line.c index 5b84a8e14..06274bee3 100644 --- a/kitty/line.c +++ b/kitty/line.c @@ -52,7 +52,7 @@ cell_text(CPUCell *cell) { static inline index_type find_colon_slash(Line *self, index_type x, index_type limit) { // Find :// at or before x - index_type pos = x; + index_type pos = MIN(x, self->xnum - 1); enum URL_PARSER_STATES {ANY, FIRST_SLASH, SECOND_SLASH}; enum URL_PARSER_STATES state = ANY; limit = MAX(2u, limit); @@ -108,7 +108,6 @@ has_url_prefix_at(Line *self, index_type at, index_type min_prefix_len, index_ty static inline bool has_url_beyond(Line *self, index_type x) { - if (self->xnum <= x + MIN_URL_LEN + 3) return false; for (index_type i = x; i < MIN(x + MIN_URL_LEN + 3, self->xnum); i++) { if (!is_url_char(self->cpu_cells[i].ch)) return false; } @@ -122,7 +121,7 @@ line_url_start_at(Line *self, index_type x) { if (x >= self->xnum || self->xnum <= MIN_URL_LEN + 3) return self->xnum; index_type ds_pos = 0, t; // First look for :// ahead of x - if (self->xnum - x > OPT(url_prefixes).max_prefix_len + 3) ds_pos = find_colon_slash(self, x + OPT(url_prefixes).max_prefix_len + 3, x < 2 ? 0 : x - 2); + ds_pos = find_colon_slash(self, x + OPT(url_prefixes).max_prefix_len + 3, x < 2 ? 0 : x - 2); if (ds_pos != 0 && has_url_beyond(self, ds_pos)) { if (has_url_prefix_at(self, ds_pos, ds_pos > x ? ds_pos - x: 0, &t)) return t; } diff --git a/kitty/mouse.c b/kitty/mouse.c index 349484bf9..8680da293 100644 --- a/kitty/mouse.c +++ b/kitty/mouse.c @@ -288,88 +288,11 @@ extend_selection(Window *w, bool ended) { } } -static inline void -extend_url(Screen *screen, Line *line, index_type *x, index_type *y, char_type sentinel) { - unsigned int count = 0; - while(count++ < 10) { - if (*x != line->xnum - 1) break; - bool next_line_starts_with_url_chars = false; - line = screen_visual_line(screen, *y + 2); - if (line) next_line_starts_with_url_chars = line_startswith_url_chars(line); - line = screen_visual_line(screen, *y + 1); - if (!line) break; - // we deliberately allow non-continued lines as some programs, like - // mutt split URLs with newlines at line boundaries - index_type new_x = line_url_end_at(line, 0, false, sentinel, next_line_starts_with_url_chars); - if (!new_x && !line_startswith_url_chars(line)) break; - *y += 1; *x = new_x; - } -} - -static inline char_type -get_url_sentinel(Line *line, index_type url_start) { - char_type before = 0, sentinel; - if (url_start > 0 && url_start < line->xnum) before = line->cpu_cells[url_start - 1].ch; - switch(before) { - case '"': - case '\'': - case '*': - sentinel = before; break; - case '(': - sentinel = ')'; break; - case '[': - sentinel = ']'; break; - case '{': - sentinel = '}'; break; - case '<': - sentinel = '>'; break; - default: - sentinel = 0; break; - } - return sentinel; -} - static inline void set_mouse_cursor_for_screen(Screen *screen) { mouse_cursor_shape = screen->modes.mouse_tracking_mode == NO_TRACKING ? OPT(default_pointer_shape): OPT(pointer_shape_when_grabbed); } -static inline void -detect_url(Screen *screen, unsigned int x, unsigned int y) { - bool has_url = false; - index_type url_start, url_end = 0; - Line *line = screen_visual_line(screen, y); - if (line->cpu_cells[x].hyperlink_id) { - mouse_cursor_shape = HAND; - screen_mark_hyperlink(screen, x, y); - return; - } - char_type sentinel; - if (line) { - url_start = line_url_start_at(line, x); - sentinel = get_url_sentinel(line, url_start); - if (url_start < line->xnum) { - bool next_line_starts_with_url_chars = false; - if (y < screen->lines - 1) { - line = screen_visual_line(screen, y+1); - next_line_starts_with_url_chars = line_startswith_url_chars(line); - line = screen_visual_line(screen, y); - } - url_end = line_url_end_at(line, x, true, sentinel, next_line_starts_with_url_chars); - } - has_url = url_end > url_start; - } - if (has_url) { - mouse_cursor_shape = HAND; - index_type y_extended = y; - extend_url(screen, line, &url_end, &y_extended, sentinel); - screen_mark_url(screen, url_start, y, url_end, y_extended); - } else { - set_mouse_cursor_for_screen(screen); - screen_mark_url(screen, 0, 0, 0, 0); - } -} - static inline void handle_mouse_movement_in_kitty(Window *w, int button, bool mouse_cell_changed) { Screen *screen = w->render_data.screen; @@ -383,6 +306,12 @@ handle_mouse_movement_in_kitty(Window *w, int button, bool mouse_cell_changed) { } +static void +detect_url(Screen *screen, unsigned int x, unsigned int y) { + if (screen_detect_url(screen, x, y)) mouse_cursor_shape = HAND; + else set_mouse_cursor_for_screen(screen); +} + HANDLER(handle_move_event) { modifiers &= ~GLFW_LOCK_MASK; unsigned int x = 0, y = 0; diff --git a/kitty/screen.c b/kitty/screen.c index a00c65289..d05cc4246 100644 --- a/kitty/screen.c +++ b/kitty/screen.c @@ -2118,6 +2118,86 @@ deactivate_overlay_line(Screen *self) { } +// }}} + +// URLs {{{ +static void +extend_url(Screen *screen, Line *line, index_type *x, index_type *y, char_type sentinel) { + unsigned int count = 0; + while(count++ < 10) { + if (*x != line->xnum - 1) break; + bool next_line_starts_with_url_chars = false; + line = screen_visual_line(screen, *y + 2); + if (line) next_line_starts_with_url_chars = line_startswith_url_chars(line); + line = screen_visual_line(screen, *y + 1); + if (!line) break; + // we deliberately allow non-continued lines as some programs, like + // mutt split URLs with newlines at line boundaries + index_type new_x = line_url_end_at(line, 0, false, sentinel, next_line_starts_with_url_chars); + if (!new_x && !line_startswith_url_chars(line)) break; + *y += 1; *x = new_x; + } +} + +static char_type +get_url_sentinel(Line *line, index_type url_start) { + char_type before = 0, sentinel; + if (url_start > 0 && url_start < line->xnum) before = line->cpu_cells[url_start - 1].ch; + switch(before) { + case '"': + case '\'': + case '*': + sentinel = before; break; + case '(': + sentinel = ')'; break; + case '[': + sentinel = ']'; break; + case '{': + sentinel = '}'; break; + case '<': + sentinel = '>'; break; + default: + sentinel = 0; break; + } + return sentinel; +} + +bool +screen_detect_url(Screen *screen, unsigned int x, unsigned int y) { + bool has_url = false; + index_type url_start, url_end = 0; + Line *line = screen_visual_line(screen, y); + if (line->cpu_cells[x].hyperlink_id) { + screen_mark_hyperlink(screen, x, y); + return true; + } + char_type sentinel; + if (line) { + url_start = line_url_start_at(line, x); + if (url_start < line->xnum) { + bool next_line_starts_with_url_chars = false; + if (y < screen->lines - 1) { + line = screen_visual_line(screen, y+1); + next_line_starts_with_url_chars = line_startswith_url_chars(line); + line = screen_visual_line(screen, y); + } + sentinel = get_url_sentinel(line, url_start); + url_end = line_url_end_at(line, x, true, sentinel, next_line_starts_with_url_chars); + } + has_url = url_end > url_start; + } + if (has_url) { + index_type y_extended = y; + extend_url(screen, line, &url_end, &y_extended, sentinel); + screen_mark_url(screen, url_start, y, url_end, y_extended); + } else { + screen_mark_url(screen, 0, 0, 0, 0); + } + return has_url; +} + + + // }}} // Python interface {{{ @@ -2404,6 +2484,7 @@ WRAP0(tab) WRAP0(linefeed) WRAP0(carriage_return) WRAP2(set_margins, 1, 1) +WRAP2(detect_url, 0, 0) WRAP0(rescale_images) static PyObject* @@ -2999,6 +3080,7 @@ static PyMethodDef methods[] = { MND(mark_as_dirty, METH_NOARGS) MND(resize, METH_VARARGS) MND(set_margins, METH_VARARGS) + MND(detect_url, METH_VARARGS) MND(rescale_images, METH_NOARGS) MND(current_key_encoding_flags, METH_NOARGS) MND(text_for_selection, METH_NOARGS) diff --git a/kitty/screen.h b/kitty/screen.h index 25a836301..4d95193d5 100644 --- a/kitty/screen.h +++ b/kitty/screen.h @@ -232,6 +232,7 @@ void screen_pop_key_encoding_flags(Screen *self, uint32_t num); uint8_t screen_current_key_encoding_flags(Screen *self); void screen_report_key_encoding_flags(Screen *self); void screen_xtmodkeys(Screen *self, uint32_t p1, uint32_t p2); +bool screen_detect_url(Screen *screen, unsigned int x, unsigned int y); #define DECLARE_CH_SCREEN_HANDLER(name) void screen_##name(Screen *screen); DECLARE_CH_SCREEN_HANDLER(bell) DECLARE_CH_SCREEN_HANDLER(backspace) diff --git a/kitty/unicode-data.h b/kitty/unicode-data.h index 7054b8815..54c599552 100644 --- a/kitty/unicode-data.h +++ b/kitty/unicode-data.h @@ -19,10 +19,7 @@ is_url_char(uint32_t ch) { static inline bool can_strip_from_end_of_url(uint32_t ch) { // remove trailing punctuation - return ( - (is_P_category(ch) && ch != '/' && ch != '&' && ch != '-') || - ch == '>' - ) ? true : false; + return (is_P_category(ch) && ch != '/' && ch != '&' && ch != '-' && ch != ')' && ch != ']' && ch != '}'); } static inline bool diff --git a/kitty_tests/datatypes.py b/kitty_tests/datatypes.py index 68c744bdc..26430fb51 100644 --- a/kitty_tests/datatypes.py +++ b/kitty_tests/datatypes.py @@ -243,13 +243,16 @@ class TestDataTypes(BaseTest): l0 = create('file:///etc/test') self.ae(l0.url_start_at(0), 0) - for trail in '.,]>)\\': + for trail in '.,\\': lx = create("http://xyz.com" + trail) self.ae(lx.url_end_at(0), len(lx) - 2) + for trail in ')}]>': + lx = create("http://xyz.com" + trail) + self.ae(lx.url_end_at(0), len(lx) - 1) l0 = create("ftp://abc/") self.ae(l0.url_end_at(0), len(l0) - 1) l2 = create("http://-abcd] ") - self.ae(l2.url_end_at(0), len(l2) - 3) + self.ae(l2.url_end_at(0), len(l2) - 2) l3 = create("http://ab.de ") self.ae(l3.url_start_at(4), 0) self.ae(l3.url_start_at(5), 0) diff --git a/kitty_tests/screen.py b/kitty_tests/screen.py index 867a66e98..d8153d8b5 100644 --- a/kitty_tests/screen.py +++ b/kitty_tests/screen.py @@ -864,3 +864,28 @@ class TestScreen(BaseTest): w('#P') w('#R') ac(9, 10) + + def test_detect_url(self): + s = self.create_screen(cols=30) + + def ae(expected, x=3, y=0): + s.detect_url(x, y) + url = ''.join(s.text_for_marked_url()) + self.assertEqual(expected, url) + + def t(url, x=0, y=0, before='', after=''): + s.reset() + s.cursor.x = x + s.cursor.y = y + s.draw(before + url + after) + ae(url, x=x + 1 + len(before), y=y) + + t('http://moo.com') + t('http://moo.com/something?else=+&what-') + for (st, e) in '() {} [] <>'.split(): + t('http://moo.com', before=st, after=e) + for trailer in ')-=]}': + t('http://moo.com' + trailer) + for trailer in '{([': + t('http://moo.com', after=trailer) + t('http://moo.com', x=s.columns - 9)