Re: [PATCH 1/4] screen-redraw.c: Improve names of border related functions.
7 views
Skip to first unread message
Marcel Partap
Jun 29, 2019, 9:53:22 AM6/29/19
to tmux-users
On 28/10/2013 00:15, Thomas Adam wrote:
> On Sun, Oct 27, 2013 at 11:37:02PM +0100, Marcel Partap wrote:
>> Rename
>> screen_redraw_cell_border1() => screen_redraw_get_border_rel()
>> screen_redraw_cell_border() => screen_redraw_cell_is_border()
>> screen_redraw_check_cell() => screen_redraw_get_cell_type()
>> screen_redraw_check_active() => screen_redraw_check_active_pane_indicator()
>
> As Nicholas has mentioned, this does much more than just rename some
> functions, and neither does this, nor any of the other three patches you've
> sent through, indicate any rationale for this change.
To get back to this: the functions were renamed (and subsequently restructured) to not do multiple unrelated things and better align name with what they do. When doing this, of course I grepped the source for all invocations of these functions to be sure to not break anything.
I totally should have documented this better, sorry.
> IIRC from an earlier round, these patches were designed to somehow "better"
> indicate which pane has the focus? If that's true, we still have the
> split-pane colourisation thing which seems to do this, albeit perhaps not as
> intuitively.
Well yes, somehow better, and I still favor it, to the point where I propose it should be the new default. After having used both styles for several years now.
So I'll try reapplying the patch series and put forward again.
Best Regards,
#marcel
> On Sun, Oct 27, 2013 at 11:37:02PM +0100, Marcel Partap wrote:
>> Rename
>> screen_redraw_cell_border1() => screen_redraw_get_border_rel()
>> screen_redraw_cell_border() => screen_redraw_cell_is_border()
>> screen_redraw_check_cell() => screen_redraw_get_cell_type()
>> screen_redraw_check_active() => screen_redraw_check_active_pane_indicator()
>
> As Nicholas has mentioned, this does much more than just rename some
> functions, and neither does this, nor any of the other three patches you've
> sent through, indicate any rationale for this change.
To get back to this: the functions were renamed (and subsequently restructured) to not do multiple unrelated things and better align name with what they do. When doing this, of course I grepped the source for all invocations of these functions to be sure to not break anything.
I totally should have documented this better, sorry.
> IIRC from an earlier round, these patches were designed to somehow "better"
> indicate which pane has the focus? If that's true, we still have the
> split-pane colourisation thing which seems to do this, albeit perhaps not as
> intuitively.
Well yes, somehow better, and I still favor it, to the point where I propose it should be the new default. After having used both styles for several years now.
So I'll try reapplying the patch series and put forward again.
Best Regards,
#marcel
Marcel Partap
Jun 29, 2019, 9:54:36 AM6/29/19
to Nicholas Marriott, tmux-users
On 27/10/2013 23:53, Nicholas Marriott wrote:
> This seems to do more than just rename?
Yes, sorry for the poor documentation.
> Don't like get_border_rel maybe is_pane_border?
Well the relation contains another piece of information: the direction the active pane is at.
> Also last one is too long.
how about screen_redraw_pane_indicator() ?
…
> -------- Original message --------
> From: Marcel Partap <mpa...@gmx.net>
> Date: 27/10/2013 22:37 (GMT+00:00)
> To: tmux-...@lists.sourceforge.net
> Subject: [PATCH 1/4] screen-redraw.c: Improve names of border related functions.
>
>
> Rename
> screen_redraw_cell_border1() => screen_redraw_get_border_rel()
> screen_redraw_cell_border() => screen_redraw_cell_is_border()
> screen_redraw_check_cell() => screen_redraw_get_cell_type()
> screen_redraw_check_active() => screen_redraw_check_active_pane_indicator()
> ---
> screen-redraw.c | 67 +++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/screen-redraw.c b/screen-redraw.c
> index a7bf81f..f5ffca2 100644
> --- a/screen-redraw.c
> +++ b/screen-redraw.c
> @@ -22,11 +22,11 @@
>
> #include "tmux.h"
>
> -int screen_redraw_cell_border1(struct window_pane *, u_int, u_int);
> -int screen_redraw_cell_border(struct client *, u_int, u_int);
> -int screen_redraw_check_cell(struct client *, u_int, u_int,
> +int screen_redraw_get_border_rel(struct window_pane *, u_int, u_int);
> +int screen_redraw_cell_is_border(struct client *, u_int, u_int);
> +int screen_redraw_get_cell_type(struct client *, u_int, u_int,
> struct window_pane **);
> -int screen_redraw_check_active(u_int, u_int, int, struct window *,
> +int screen_redraw_check_active_pane_indicator(u_int, u_int, int, struct window *,
> struct window_pane *);
>
> void screen_redraw_draw_number(struct client *, struct window_pane *);
> @@ -47,57 +47,62 @@ void screen_redraw_draw_number(struct client *, struct window_pane *);
>
> #define CELL_BORDERS " xqlkmjwvtun~"
>
> -/* Check if cell is on the border of a particular pane. */
> +#define BORDER_NOT 0
> +#define BORDER_LEFT 1
> +#define BORDER_RIGHT 2
> +#define BORDER_TOP 4
> +#define BORDER_BOTTOM 8
> +
> +/* Check if cell is on border of given pane and how it relates to it. */
> int
> -screen_redraw_cell_border1(struct window_pane *wp, u_int px, u_int py)
> +screen_redraw_get_border_rel(struct window_pane *wp, u_int px, u_int py)
> {
> /* Inside pane. */
> if (px >= wp->xoff && px < wp->xoff + wp->sx &&
> py >= wp->yoff && py < wp->yoff + wp->sy)
> - return (0);
> + return (BORDER_NOT);
>
> /* Left/right borders. */
> if ((wp->yoff == 0 || py >= wp->yoff - 1) && py <= wp->yoff + wp->sy) {
> if (wp->xoff != 0 && px == wp->xoff - 1)
> - return (1);
> + return (BORDER_LEFT);
> if (px == wp->xoff + wp->sx)
> - return (1);
> + return (BORDER_RIGHT);
> }
>
> /* Top/bottom borders. */
> if ((wp->xoff == 0 || px >= wp->xoff - 1) && px <= wp->xoff + wp->sx) {
> if (wp->yoff != 0 && py == wp->yoff - 1)
> - return (1);
> + return (BORDER_TOP);
> if (py == wp->yoff + wp->sy)
> - return (1);
> + return (BORDER_BOTTOM);
> }
>
> /* Outside pane. */
> - return (-1);
> + return (BORDER_NOT);
> }
>
> -/* Check if a cell is on the pane border. */
> +/* Check if a cell is part of any pane border. */
> int
> -screen_redraw_cell_border(struct client *c, u_int px, u_int py)
> +screen_redraw_cell_is_border(struct client *c, u_int px, u_int py)
> {
> struct window *w = c->session->curw->window;
> struct window_pane *wp;
> - int retval;
>
> /* Check all the panes. */
> TAILQ_FOREACH(wp, &w->panes, entry) {
> if (!window_pane_visible(wp))
> continue;
> - if ((retval = screen_redraw_cell_border1(wp, px, py)) != -1)
> - return (retval);
> + if (screen_redraw_get_border_rel(wp, px, py) != BORDER_NOT)
> + return (1);
> }
>
> return (0);
> }
>
> -/* Check if cell inside a pane. */
> +/* Check if cell is inside given pane or on its border. */
> int
> -screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> +screen_redraw_get_cell_type(struct client *c, u_int px, u_int py,
> struct window_pane **wpp)
> {
> struct window *w = c->session->curw->window;
> @@ -120,7 +125,7 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> continue;
>
> /* If definitely inside, return so. */
> - if (!screen_redraw_cell_border(c, px, py))
> + if (!screen_redraw_cell_is_border(c, px, py))
> return (CELL_INSIDE);
>
> /*
> @@ -128,13 +133,13 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> * 4), right, top, and bottom (bit 1) of this cell are borders.
> */
> borders = 0;
> - if (px == 0 || screen_redraw_cell_border(c, px - 1, py))
> + if (px == 0 || screen_redraw_cell_is_border(c, px - 1, py))
> borders |= 8;
> - if (px <= w->sx && screen_redraw_cell_border(c, px + 1, py))
> + if (px <= w->sx && screen_redraw_cell_is_border(c, px + 1, py))
> borders |= 4;
> - if (py == 0 || screen_redraw_cell_border(c, px, py - 1))
> + if (py == 0 || screen_redraw_cell_is_border(c, px, py - 1))
> borders |= 2;
> - if (py <= w->sy && screen_redraw_cell_border(c, px, py + 1))
> + if (py <= w->sy && screen_redraw_cell_is_border(c, px, py + 1))
> borders |= 1;
>
> /*
> @@ -174,11 +179,11 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
>
> /* Check active pane indicator. */
> int
> -screen_redraw_check_active(u_int px, u_int py, int type, struct window *w,
> +screen_redraw_check_active_pane_indicator(u_int px, u_int py, int type, struct window *w,
> struct window_pane *wp)
> {
> /* Is this off the active pane border? */
> - if (screen_redraw_cell_border1(w->active, px, py) != 1)
> + if (screen_redraw_get_border_rel(w->active, px, py) == BORDER_NOT)
> return (0);
>
> /* If there are more than two panes, that's enough. */
> @@ -223,7 +228,7 @@ screen_redraw_screen(struct client *c, int status_only, int borders_only)
> struct tty *tty = &c->tty;
> struct window_pane *wp;
> struct grid_cell active_gc, other_gc;
> - u_int i, j, type, top;
> + u_int i, j, celltype, top;
> int status, spos, fg, bg;
>
> /* Suspended clients should not be updated. */
> @@ -272,15 +277,15 @@ screen_redraw_screen(struct client *c, int status_only, int borders_only)
> break;
> }
> for (i = 0; i < tty->sx; i++) {
> - type = screen_redraw_check_cell(c, i, j, &wp);
> - if (type == CELL_INSIDE)
> + celltype = screen_redraw_get_cell_type(c, i, j, &wp);
> + if (celltype == CELL_INSIDE)
> continue;
> - if (screen_redraw_check_active(i, j, type, w, wp))
> + if (screen_redraw_check_active_pane_indicator(i, j, celltype, w, wp))
> tty_attributes(tty, &active_gc);
> else
> tty_attributes(tty, &other_gc);
> tty_cursor(tty, i, top + j);
> - tty_putc(tty, CELL_BORDERS[type]);
> + tty_putc(tty, CELL_BORDERS[celltype]);
> }
> }
>
> --
> 1.8.4
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
> _______________________________________________
> tmux-users mailing list
> tmux-...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tmux-users
> This seems to do more than just rename?
Yes, sorry for the poor documentation.
> Don't like get_border_rel maybe is_pane_border?
Well the relation contains another piece of information: the direction the active pane is at.
> Also last one is too long.
how about screen_redraw_pane_indicator() ?
…
> -------- Original message --------
> From: Marcel Partap <mpa...@gmx.net>
> Date: 27/10/2013 22:37 (GMT+00:00)
> To: tmux-...@lists.sourceforge.net
> Subject: [PATCH 1/4] screen-redraw.c: Improve names of border related functions.
>
>
> Rename
> screen_redraw_cell_border1() => screen_redraw_get_border_rel()
> screen_redraw_cell_border() => screen_redraw_cell_is_border()
> screen_redraw_check_cell() => screen_redraw_get_cell_type()
> screen_redraw_check_active() => screen_redraw_check_active_pane_indicator()
> screen-redraw.c | 67 +++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/screen-redraw.c b/screen-redraw.c
> index a7bf81f..f5ffca2 100644
> --- a/screen-redraw.c
> +++ b/screen-redraw.c
> @@ -22,11 +22,11 @@
>
> #include "tmux.h"
>
> -int screen_redraw_cell_border1(struct window_pane *, u_int, u_int);
> -int screen_redraw_cell_border(struct client *, u_int, u_int);
> -int screen_redraw_check_cell(struct client *, u_int, u_int,
> +int screen_redraw_get_border_rel(struct window_pane *, u_int, u_int);
> +int screen_redraw_cell_is_border(struct client *, u_int, u_int);
> +int screen_redraw_get_cell_type(struct client *, u_int, u_int,
> struct window_pane **);
> -int screen_redraw_check_active(u_int, u_int, int, struct window *,
> +int screen_redraw_check_active_pane_indicator(u_int, u_int, int, struct window *,
> struct window_pane *);
>
> void screen_redraw_draw_number(struct client *, struct window_pane *);
> @@ -47,57 +47,62 @@ void screen_redraw_draw_number(struct client *, struct window_pane *);
>
> #define CELL_BORDERS " xqlkmjwvtun~"
>
> -/* Check if cell is on the border of a particular pane. */
> +#define BORDER_NOT 0
> +#define BORDER_LEFT 1
> +#define BORDER_RIGHT 2
> +#define BORDER_TOP 4
> +#define BORDER_BOTTOM 8
> +
> +/* Check if cell is on border of given pane and how it relates to it. */
> int
> -screen_redraw_cell_border1(struct window_pane *wp, u_int px, u_int py)
> +screen_redraw_get_border_rel(struct window_pane *wp, u_int px, u_int py)
> {
> /* Inside pane. */
> if (px >= wp->xoff && px < wp->xoff + wp->sx &&
> py >= wp->yoff && py < wp->yoff + wp->sy)
> - return (0);
> + return (BORDER_NOT);
>
> /* Left/right borders. */
> if ((wp->yoff == 0 || py >= wp->yoff - 1) && py <= wp->yoff + wp->sy) {
> if (wp->xoff != 0 && px == wp->xoff - 1)
> - return (1);
> + return (BORDER_LEFT);
> if (px == wp->xoff + wp->sx)
> - return (1);
> + return (BORDER_RIGHT);
> }
>
> /* Top/bottom borders. */
> if ((wp->xoff == 0 || px >= wp->xoff - 1) && px <= wp->xoff + wp->sx) {
> if (wp->yoff != 0 && py == wp->yoff - 1)
> - return (1);
> + return (BORDER_TOP);
> if (py == wp->yoff + wp->sy)
> - return (1);
> + return (BORDER_BOTTOM);
> }
>
> /* Outside pane. */
> - return (-1);
> + return (BORDER_NOT);
> }
>
> -/* Check if a cell is on the pane border. */
> +/* Check if a cell is part of any pane border. */
> int
> -screen_redraw_cell_border(struct client *c, u_int px, u_int py)
> +screen_redraw_cell_is_border(struct client *c, u_int px, u_int py)
> {
> struct window *w = c->session->curw->window;
> struct window_pane *wp;
> - int retval;
>
> /* Check all the panes. */
> TAILQ_FOREACH(wp, &w->panes, entry) {
> if (!window_pane_visible(wp))
> continue;
> - if ((retval = screen_redraw_cell_border1(wp, px, py)) != -1)
> - return (retval);
> + if (screen_redraw_get_border_rel(wp, px, py) != BORDER_NOT)
> + return (1);
> }
>
> return (0);
> }
>
> -/* Check if cell inside a pane. */
> +/* Check if cell is inside given pane or on its border. */
> int
> -screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> +screen_redraw_get_cell_type(struct client *c, u_int px, u_int py,
> struct window_pane **wpp)
> {
> struct window *w = c->session->curw->window;
> @@ -120,7 +125,7 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> continue;
>
> /* If definitely inside, return so. */
> - if (!screen_redraw_cell_border(c, px, py))
> + if (!screen_redraw_cell_is_border(c, px, py))
> return (CELL_INSIDE);
>
> /*
> @@ -128,13 +133,13 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
> * 4), right, top, and bottom (bit 1) of this cell are borders.
> */
> borders = 0;
> - if (px == 0 || screen_redraw_cell_border(c, px - 1, py))
> + if (px == 0 || screen_redraw_cell_is_border(c, px - 1, py))
> borders |= 8;
> - if (px <= w->sx && screen_redraw_cell_border(c, px + 1, py))
> + if (px <= w->sx && screen_redraw_cell_is_border(c, px + 1, py))
> borders |= 4;
> - if (py == 0 || screen_redraw_cell_border(c, px, py - 1))
> + if (py == 0 || screen_redraw_cell_is_border(c, px, py - 1))
> borders |= 2;
> - if (py <= w->sy && screen_redraw_cell_border(c, px, py + 1))
> + if (py <= w->sy && screen_redraw_cell_is_border(c, px, py + 1))
> borders |= 1;
>
> /*
> @@ -174,11 +179,11 @@ screen_redraw_check_cell(struct client *c, u_int px, u_int py,
>
> /* Check active pane indicator. */
> int
> -screen_redraw_check_active(u_int px, u_int py, int type, struct window *w,
> +screen_redraw_check_active_pane_indicator(u_int px, u_int py, int type, struct window *w,
> struct window_pane *wp)
> {
> /* Is this off the active pane border? */
> - if (screen_redraw_cell_border1(w->active, px, py) != 1)
> + if (screen_redraw_get_border_rel(w->active, px, py) == BORDER_NOT)
> return (0);
>
> /* If there are more than two panes, that's enough. */
> @@ -223,7 +228,7 @@ screen_redraw_screen(struct client *c, int status_only, int borders_only)
> struct tty *tty = &c->tty;
> struct window_pane *wp;
> struct grid_cell active_gc, other_gc;
> - u_int i, j, type, top;
> + u_int i, j, celltype, top;
> int status, spos, fg, bg;
>
> /* Suspended clients should not be updated. */
> @@ -272,15 +277,15 @@ screen_redraw_screen(struct client *c, int status_only, int borders_only)
> break;
> }
> for (i = 0; i < tty->sx; i++) {
> - type = screen_redraw_check_cell(c, i, j, &wp);
> - if (type == CELL_INSIDE)
> + celltype = screen_redraw_get_cell_type(c, i, j, &wp);
> + if (celltype == CELL_INSIDE)
> continue;
> - if (screen_redraw_check_active(i, j, type, w, wp))
> + if (screen_redraw_check_active_pane_indicator(i, j, celltype, w, wp))
> tty_attributes(tty, &active_gc);
> else
> tty_attributes(tty, &other_gc);
> tty_cursor(tty, i, top + j);
> - tty_putc(tty, CELL_BORDERS[type]);
> + tty_putc(tty, CELL_BORDERS[celltype]);
> }
> }
>
> --
> 1.8.4
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
> _______________________________________________
> tmux-users mailing list
> tmux-...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tmux-users
Nicholas Marriott
Jun 30, 2019, 3:06:40 PM6/30/19
to Marcel Partap, tmux-users
Hi
I don't know what you mean by rel/relation.
I'd go is_pane_border and is_any_border.
check_active has gone now not sure what it is now.
I don't know what you mean by rel/relation.
I'd go is_pane_border and is_any_border.
check_active has gone now not sure what it is now.
Reply all
Reply to author
Forward
0 new messages