Re: [dwm] [dvtm] scrollback and 256color support
On Mon, Aug 25, 2008 at 11:58:45PM -0700, Donald Chai wrote:
> Hi, the following two patches add support for a scrollback buffer and
> enhanced color in dvtm.
>
> 256color:
> - Allows the use of nice VIM color schemes
> - Ncurses doesn't allow more than 256 fg/bg color pairs on screen at
> the same time.
> - 8 or 256 colors only. If your terminal supports 88 colors, you need
> to
> set up your own variant of rxvt, e.g. rxvt-88color
Ok, i finally found the time to take a *quick* look at the color patch,
see some remarks below.
>diff --git a/config.h b/config.h
>index b2884e3..9c1387d 100644
>--- a/config.h
>+++ b/config.h
>@@ -10,24 +10,21 @@
> * A_BOLD Extra bright or bold
> * A_PROTECT Protected mode
> * A_INVIS Invisible or blank mode
>- * COLOR(fg,bg)Color where fg and bg are one of:
> *
>- * COLOR_BLACK
>- * COLOR_RED
>- * COLOR_GREEN
>- * COLOR_YELLOW
>- * COLOR_BLUE
>- * COLOR_MAGENTA
>- * COLOR_CYAN
>- * COLOR_WHITE
> */
Aren't there some macros which could be used instead of raw color numbers?
Maybe something like RGB(r, g, b) which would fall back to the most
resembling basic color.
>-#define ATTR_SELECTED COLOR(COLOR_RED,COLOR_BLACK)
>+#define ATTR_SELECTED A_NORMAL
>+#define FG_SELECTED COLORS==256 ? 68 : COLOR_BLUE
>+#define BG_SELECTED -1
> /* curses attributes for normal (not selected) windows */
> #define ATTR_NORMAL A_NORMAL
>+#define FG_NORMAL -1
>+#define BG_NORMAL -1
> /* status bar (command line option -s) position */
> #define BARPOSBarTop /* BarBot, BarOff */
> /* curses attributes for the status bar */
>-#define BAR_ATTRCOLOR(COLOR_RED,COLOR_BLACK)
>+#define BAR_ATTRA_NORMAL
>+#define FG_BAR COLOR_RED
>+#define BG_BAR -1
Just a minor detail, i will probably rename those so that they share a
common prefix BAR_{ATTR,FG,BG}, NORMAL_{ATTR,FG,BG} etc.
>@@ -459,9 +470,11 @@ static void interpret_csi_ICH(madtty_t *t, int param[],
>int pcount)
> for (i = t->cols - 1; i >= t->curs_col + n; i--) {
> row->text[i] = row->text[i - n];
> row->attr[i] = row->attr[i - n];
>+row->bg [i] = row->fg [i - n];
Is this a typo or not?
>+row->fg [i] = row->fg [i - n];
> }
>@@ -478,16 +491,18 @@ static void interpret_csi_DCH(madtty_t *t, int param[],
>int pcount)
> for (i = t->curs_col; i < t->cols - n; i++) {
> row->text[i] = row->text[i + n];
> row->attr[i] = row->attr[i + n];
>+row->bg [i] = row->fg [i + n];
Same here.
>+row->fg [i] = row->fg [i + n];
> }
> void madtty_init_colors(void)
> {
>-if (COLOR_PAIRS > 64) {
>+use_palette = 0;
>+
>+if (1 && COLORS >= 256 && COLOR_PAIRS >= 256) {
^
` not needed
>+use_palette = 1;
>+use_default_colors();
>+has_default = 1;
>+
@@ -1231,22 +1337,6 @@ void madtty_init_colors(void)
}
}
>-int madtty_color_pair(int fg, int bg)
>-{
>-if (has_default) {
>-if (fg < -1)
>-fg = -1;
>-if (bg < -1)
>-bg = -1;
>-return COLOR_PAIR((fg + 1) * 16 + bg + 1);
>-}
>-if (fg < 0)
>-fg = COLOR_WHITE;
>-if (bg < 0)
>-bg = COLOR_BLACK;
>-return COLOR_PAIR((7 - fg) * 8 + bg);
>-}
>-
> void madtty_set_handler(madtty_t *t, madtty_handler_t handler)
> {
> t->handler = handler;
>diff --git a/madtty.h b/madtty.h
>index 9445526..a30acee 100644
>--- a/madtty.h
>+++ b/madtty.h
>@@ -71,6 +71,7 @@ void madtty_keypress(madtty_t *, int keycode);
> void madtty_keypress_sequence(madtty_t *, const char *seq);
> void madtty_dirty(madtty_t *t);
> void madtty_draw(madtty_t *, WINDOW *win, int startrow, int startcol);
>+void madtty_color_set(WINDOW *win, short fg, short bg);
You didn't remove madtty_color_pair from the header file.
I tested it with script from[1] and it seems to work. The other
script[2] from that page triggers an assertion / results in a segfault.
This also happens without your patch but it would neverthless be good to
fix it.
Thanks,
Marc
[1] http://www.frexx.de/xterm-256-notes/data/xterm-colortest
[2] http://www.frexx.de/xterm-256-notes/data/256colors2.pl
--
Marc Andre Tanner >< http://www.brain-dump.org/ >< GPG key: CF7D56C0
Re: [dwm] [dvtm] scrollback and 256color support
On Thu, Aug 28, 2008 at 03:56:04PM -0700, Donald Chai wrote:
>> Thanks for the patches, scrollback is probably the most requested
>> dvtm feature. You introduced a separate buffer for the scrollback
>> data, did you consider allocating more space for the main buffer and
>> just adjusting the scroll_{top,bot} pointers and using them in
>> madtty_draw to draw only the currently visible area?
>
> I did consider that, actually. Several reasons why I use a separate
> buffer:
> - From what I understand of vt100/xterm behavior, you can use DECSTBM
> to
> set the scrolling region. If the top line of the screen is *not* in the
> scrolling region, you'd need to do some extra work to have text scroll
> offscreen.
> - The scrollback buffer is a circular buffer for runtime efficiency,
> while the screen is a regular array.
> - I'd like to add some kind of compression for the scrollback buffer
> (e.g. run length encoding for character attributes, convert from wchar_t
> using wcsrtombs).
Ok i see, makes sense. I have commited your patch and added the
possibility to set the size of the scroll back history buffer at
runtime. Hope I didn't screw it up ;)
Thanks,
Marc
--
Marc Andre Tanner >< http://www.brain-dump.org/ >< GPG key: CF7D56C0
Re: [dwm] [dvtm] scrollback and 256color support
Thanks for the patches, scrollback is probably the most requested
dvtm feature. You introduced a separate buffer for the scrollback
data, did you consider allocating more space for the main buffer and
just adjusting the scroll_{top,bot} pointers and using them in
madtty_draw to draw only the currently visible area?
I did consider that, actually. Several reasons why I use a separate
buffer:
- From what I understand of vt100/xterm behavior, you can use
DECSTBM to set the scrolling region. If the top line of the screen is
*not* in the scrolling region, you'd need to do some extra work to
have text scroll offscreen.
- The scrollback buffer is a circular buffer for runtime efficiency,
while the screen is a regular array.
- I'd like to add some kind of compression for the scrollback buffer
(e.g. run length encoding for character attributes, convert from
wchar_t using wcsrtombs).
Re: [dwm] [dvtm] scrollback and 256color support
On Mon, Aug 25, 2008 at 11:58:45PM -0700, Donald Chai wrote:
> Hi, the following two patches add support for a scrollback buffer and
> enhanced color in dvtm.
>
> scrollback:
> - Simply set up some keybinding to scrollback(), passing in "-1" or "1"
> to scroll up or down.
Thanks for the patches, scrollback is probably the most requested
dvtm feature. You introduced a separate buffer for the scrollback
data, did you consider allocating more space for the main buffer and
just adjusting the scroll_{top,bot} pointers and using them in
madtty_draw to draw only the currently visible area?
Another detail, it would be nice if the buffer size could be specified
with an additional parameter to madtty_create. This could then be used
to set the scrollback buffer size from a command line option.
I need to get some sleep now, i will later take a look at the second
patch.
Thanks,
Marc
--
Marc Andre Tanner >< http://www.brain-dump.org/ >< GPG key: CF7D56C0
[dwm] [dvtm] scrollback and 256color support
Hi, the following two patches add support for a scrollback buffer and enhanced color in dvtm. scrollback: - Simply set up some keybinding to scrollback(), passing in "-1" or "1" to scroll up or down. 256color: - Allows the use of nice VIM color schemes - Ncurses doesn't allow more than 256 fg/bg color pairs on screen at the same time. - 8 or 256 colors only. If your terminal supports 88 colors, you need to set up your own variant of rxvt, e.g. rxvt-88color scrollback.diff Description: Binary data 256color.diff Description: Binary data
