|
From: Haakon R. <haa...@fy...> - 2006-10-03 10:06:57
|
First of all, I'd like to thank you guys for making CGDB: Finally
I have a good debugger with a source view and syntax highlighting
without a bloated and buggy GUI interface! :-)
Moving onward, I have one small bug report and a suggested
enhancement (patch included).
First the bug: When doing 'make install DESTDIR=/foo', the
installer also creates the directory /foo.. and copies
gdbmi_driver into /foo../progs/. Looking at the makefiles in
lib/gdbmi/src/, I see that the problem comes from
noinst_bindir = $(top_builddir)/progs
$(top_builddir) is '..', and appended to $(DESTDIR), this
gives the described result. I haven't included a fix since
I don't know if gdbmi_driver should be installed in the bin
directory along with cgdb.
As for my enhancement, I was wondering if you'd consider making
it an option to highlight the entire current line when scrolling
around in the source window. The current behavior is to make the
current line number bold, but I find it much easier to identify the
current line when the highlighting extends all the way across the
screen.
I've included a patch below that implements this feature. Currently,
it just replaces the old behavior, since that's all I need for my own
personal use. If you'd like to push this upstream, but not in its
current state, let me know -- I'll gladly make the required changes.
--- cgdb-0.6.3/cgdb/src/highlight_groups.c.orig 2006-06-03 20:09:06.000000000 +0200
+++ cgdb-0.6.3/cgdb/src/highlight_groups.c 2006-10-03 09:28:58.000000000 +0200
@@ -93,6 +93,7 @@
{HLG_ENABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_RED, COLOR_BLACK},
{HLG_DISABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_YELLOW, COLOR_BLACK},
{HLG_LOGO, A_BOLD, A_BOLD, COLOR_BLUE, COLOR_BLACK},
+ {HLG_CURRENT_LINE, A_NORMAL, A_NORMAL, COLOR_BLACK, COLOR_WHITE},
{HLG_LAST, A_NORMAL, A_NORMAL, -1, -1}
};
@@ -115,6 +116,7 @@
{HLG_ENABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_RED, -1},
{HLG_DISABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_YELLOW, -1},
{HLG_LOGO, A_BOLD, A_BOLD, COLOR_BLUE, -1},
+ {HLG_CURRENT_LINE, A_REVERSE, A_REVERSE, -1, -1},
{HLG_LAST, A_NORMAL, A_NORMAL, -1, -1}
};
@@ -140,6 +142,7 @@
{HLG_ENABLED_BREAKPOINT, "Breakpoint"},
{HLG_DISABLED_BREAKPOINT, "DisabledBreakpoint"},
{HLG_LOGO, "Logo"},
+ {HLG_CURRENT_LINE, "CurrentLine"},
{HLG_LAST, "NULL"}
};
--- cgdb-0.6.3/cgdb/src/sources.c.orig 2006-06-03 20:09:06.000000000 +0200
+++ cgdb-0.6.3/cgdb/src/sources.c 2006-10-03 09:24:47.000000000 +0200
@@ -675,22 +675,22 @@
/* Ordinary lines */
else{
if ( focus && sview->cur->sel_line == line )
- wattron(sview->win, A_BOLD);
-
+ {
+ if (hl_groups_get_attr (hl_groups_instance, HLG_CURRENT_LINE, &attr) == -1)
+ return -1;
+ wattron(sview->win, attr);
+ }
wprintw(sview->win, fmt, line+1);
-
- if ( focus && sview->cur->sel_line == line )
- wattroff(sview->win, A_BOLD);
- if (focus)
+ if (focus && sview->cur->sel_line != line)
wattron(sview->win, A_BOLD);
waddch(sview->win, VERT_LINE);
- if (focus)
+ if (focus && sview->cur->sel_line != line)
wattroff(sview->win, A_BOLD);
waddch(sview->win, ' ');
/* I know this is rediculous, it needs to be reimplemented */
- if ( sources_syntax_on ) {
+ if (!(focus && sview->cur->sel_line == line) && sources_syntax_on) {
/* No special line information */
if (line == sview->cur->sel_line &&
sview->cur->buf.cur_line != NULL) {
@@ -714,6 +714,9 @@
width-lwidth-2, sview->cur->sel_col);
}
}
+
+ if (focus && sview->cur->sel_line == line)
+ wattroff(sview->win, attr);
}
} else {
wprintw(sview->win, "%s\n", sview->cur->buf.tlines[line]);
--- cgdb-0.6.3/cgdb/include/highlight_groups.h.orig 2006-06-03 20:09:05.000000000 +0200
+++ cgdb-0.6.3/cgdb/include/highlight_groups.h 2006-10-03 09:17:10.000000000 +0200
@@ -41,6 +41,7 @@
HLG_ENABLED_BREAKPOINT,
HLG_DISABLED_BREAKPOINT,
HLG_LOGO,
+ HLG_CURRENT_LINE,
HLG_LAST
};
--
Haakon
|
|
From: Bob R. <bob...@co...> - 2006-10-03 20:21:27
|
On Tue, Oct 03, 2006 at 12:06:42PM +0200, Haakon Riiser wrote: > First of all, I'd like to thank you guys for making CGDB: Finally > I have a good debugger with a source view and syntax highlighting > without a bloated and buggy GUI interface! :-) Thanks. > Moving onward, I have one small bug report and a suggested > enhancement (patch included). > > > First the bug: When doing 'make install DESTDIR=/foo', the > installer also creates the directory /foo.. and copies > gdbmi_driver into /foo../progs/. Looking at the makefiles in > lib/gdbmi/src/, I see that the problem comes from This should be fixed in SVN now. Thanks for the report. > As for my enhancement, I was wondering if you'd consider making > it an option to highlight the entire current line when scrolling > around in the source window. The current behavior is to make the > current line number bold, but I find it much easier to identify the > current line when the highlighting extends all the way across the > screen. > > I've included a patch below that implements this feature. Currently, > it just replaces the old behavior, since that's all I need for my own > personal use. If you'd like to push this upstream, but not in its > current state, let me know -- I'll gladly make the required changes. This seems like a reasonable idea. I'm wondering how this is best implemented though. Currently, CGDB only highlights the line number. This patch would have it highlight the entire line. How would the user switch back and forth between this functionality? I applied your patch, and noticed that when breakpoints are set, and you go over them, the current line is no longer highlighted. I also noticed that if I do a search, cgdb segfaults. I don't have time to look into these but if you want to think about how this option could nicely fit into CGDB from a users perspective, that would be useful. Mike, any opinions? Thanks, Bob Rossi |
|
From: Haakon R. <haa...@fy...> - 2006-10-05 16:34:19
|
[Bob Rossi]
>> As for my enhancement, I was wondering if you'd consider making
>> it an option to highlight the entire current line when scrolling
>> around in the source window. The current behavior is to make
>> the current line number bold, but I find it much easier to
>> identify the current line when the highlighting extends all
>> the way across the screen.
> This seems like a reasonable idea. I'm wondering how this is best
> implemented though. Currently, CGDB only highlights the line number.
> This patch would have it highlight the entire line. How would the user
> switch back and forth between this functionality?
I was thinking a new option. More on this below.
> I applied your patch, and noticed that when breakpoints are set, and you
> go over them, the current line is no longer highlighted.
Yes, don't you agree that's a feature? :-)
When I first decided to change the behavior of the current line
highlighting, it was because I was scrolling the source to set
breakpoints, and found that with heavily indented code, the
boldfaced line number all the way on the left didn't help much.
When I enable a breakpoint, I appreciate immediate visual feedback,
so just allowing the normal breakpoint highlighting to work
normally seemed like a good solution to me. The minute you're
moving away from the breakpoint, the current line is highlighted
again, so I don't see what the problem is.
> I also noticed that if I do a search, cgdb segfaults.
That was because I had carelessly changed
if ( sources_syntax_on ) {
to
if (!(focus && sview->cur->sel_line == line) && sources_syntax_on) {
without considering the effects of branching into the "else"
block.
Anyway, the reason for the above change was that I wanted to
disable regular syntax highlighting for the current line. Now,
after trying it with the regular syntax highlighting enabled,
I'm not so sure that I even /want/ to disable regular syntax
highlighting. When using the reverse bit for highlighting the
current line, the syntax highlighting overlays quite nicely.
> I don't have time to look into these but if you want to think
> about how this option could nicely fit into CGDB from a users
> perspective, that would be useful.
I'm not sure what you mean. As I said above, I thought about adding
a new highlighting group for the current line (the on that is now
hardcoded to boldfaced text), and an option to let it extend across
the entire line.
By the way, here's my latest version of the patch. The only change
is the one I described above (fixing the segfault by letting the
normal highlighting overlay the current line highlighting).
--- cgdb-0.6.3/cgdb/src/sources.c.orig 2006-06-03 20:09:06.000000000 +0200
+++ cgdb-0.6.3/cgdb/src/sources.c 2006-10-05 18:04:40.829514437 +0200
@@ -675,17 +675,17 @@
/* Ordinary lines */
else{
if ( focus && sview->cur->sel_line == line )
- wattron(sview->win, A_BOLD);
-
+ {
+ if (hl_groups_get_attr (hl_groups_instance, HLG_CURRENT_LINE, &attr) == -1)
+ return -1;
+ wattron(sview->win, attr);
+ }
wprintw(sview->win, fmt, line+1);
-
- if ( focus && sview->cur->sel_line == line )
- wattroff(sview->win, A_BOLD);
- if (focus)
+ if (focus && sview->cur->sel_line != line)
wattron(sview->win, A_BOLD);
waddch(sview->win, VERT_LINE);
- if (focus)
+ if (focus && sview->cur->sel_line != line)
wattroff(sview->win, A_BOLD);
waddch(sview->win, ' ');
@@ -714,6 +714,9 @@
width-lwidth-2, sview->cur->sel_col);
}
}
+
+ if (focus && sview->cur->sel_line == line)
+ wattroff(sview->win, attr);
}
} else {
wprintw(sview->win, "%s\n", sview->cur->buf.tlines[line]);
--- cgdb-0.6.3/cgdb/src/highlight_groups.c.orig 2006-06-03 20:09:06.000000000 +0200
+++ cgdb-0.6.3/cgdb/src/highlight_groups.c 2006-10-05 17:57:10.172971689 +0200
@@ -93,6 +93,7 @@
{HLG_ENABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_RED, COLOR_BLACK},
{HLG_DISABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_YELLOW, COLOR_BLACK},
{HLG_LOGO, A_BOLD, A_BOLD, COLOR_BLUE, COLOR_BLACK},
+ {HLG_CURRENT_LINE, A_NORMAL, A_NORMAL, COLOR_BLACK, COLOR_WHITE},
{HLG_LAST, A_NORMAL, A_NORMAL, -1, -1}
};
@@ -115,6 +116,7 @@
{HLG_ENABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_RED, -1},
{HLG_DISABLED_BREAKPOINT, A_BOLD, A_BOLD, COLOR_YELLOW, -1},
{HLG_LOGO, A_BOLD, A_BOLD, COLOR_BLUE, -1},
+ {HLG_CURRENT_LINE, A_REVERSE, A_REVERSE, -1, -1},
{HLG_LAST, A_NORMAL, A_NORMAL, -1, -1}
};
@@ -140,6 +142,7 @@
{HLG_ENABLED_BREAKPOINT, "Breakpoint"},
{HLG_DISABLED_BREAKPOINT, "DisabledBreakpoint"},
{HLG_LOGO, "Logo"},
+ {HLG_CURRENT_LINE, "CurrentLine"},
{HLG_LAST, "NULL"}
};
--- cgdb-0.6.3/cgdb/include/highlight_groups.h.orig 2006-06-03 20:09:05.000000000 +0200
+++ cgdb-0.6.3/cgdb/include/highlight_groups.h 2006-10-05 17:57:10.173971501 +0200
@@ -41,6 +41,7 @@
HLG_ENABLED_BREAKPOINT,
HLG_DISABLED_BREAKPOINT,
HLG_LOGO,
+ HLG_CURRENT_LINE,
HLG_LAST
};
--
Haakon
|
|
From: Steve F. <st...@fo...> - 2006-10-05 20:29:01
|
On 10/3/06, Haakon Riiser <haa...@fy...> wrote: > As for my enhancement, I was wondering if you'd consider making > it an option to highlight the entire current line when scrolling > around in the source window. The current behavior is to make the > current line number bold, but I find it much easier to identify the > current line when the highlighting extends all the way across the > screen. I had to make a similar change myself, since the font I like (Inconsolata) doesn't have a bold version. I just made the attribute/color of the hilited line number configurable, but never submitted a patch since it was just a personal hack that didn't fit that well with the previous behavior (particularly on lines with breakpoints). Just a data point to consider... I should try Haakon's patch when I can (I'm out of town for the weekend) to see if it makes me happy. |
|
From: Bob R. <bob...@co...> - 2006-10-06 00:31:37
|
On Thu, Oct 05, 2006 at 06:34:00PM +0200, Haakon Riiser wrote: > [Bob Rossi] > > >> As for my enhancement, I was wondering if you'd consider making > >> it an option to highlight the entire current line when scrolling > >> around in the source window. The current behavior is to make > >> the current line number bold, but I find it much easier to > >> identify the current line when the highlighting extends all > >> the way across the screen. > > > This seems like a reasonable idea. I'm wondering how this is best > > implemented though. Currently, CGDB only highlights the line number. > > This patch would have it highlight the entire line. How would the user > > switch back and forth between this functionality? > > I was thinking a new option. More on this below. OK, I can agree with this. > > I applied your patch, and noticed that when breakpoints are set, and you > > go over them, the current line is no longer highlighted. > > Yes, don't you agree that's a feature? :-) I agree that some people could like this. I don't like it much, and so probably wouldn't apply it upstream as it is, but I think we can come up with something that we both like. > > I don't have time to look into these but if you want to think > > about how this option could nicely fit into CGDB from a users > > perspective, that would be useful. > > I'm not sure what you mean. As I said above, I thought about adding > a new highlighting group for the current line (the on that is now > hardcoded to boldfaced text), and an option to let it extend across > the entire line. > > By the way, here's my latest version of the patch. The only change > is the one I described above (fixing the segfault by letting the > normal highlighting overlay the current line highlighting). I misunderstood the problem that you are having. Are you saying that highlighting the line numbers with certain information isn't useful at all times because you are scrolling to the right to the point where the line numbers aren't even on the screen? Just to get an idea of what you would find useful, I have a few questions below. Would it be useful for CGDB to highlight lines with breakpoints by highlighting the entire line red or yellow (based on breakpoint enabled or disabled) instead of just the line number? Would it be useful for CGDB to highlight the current line the user is browsing by highlighting the entire line instead of just the line number? Thanks, Bob Rossi |
|
From: Haakon R. <haa...@fy...> - 2006-10-06 11:39:04
|
[Bob Rossi] >> Yes, don't you agree that's a feature? :-) > > I agree that some people could like this. I don't like it much, and so > probably wouldn't apply it upstream as it is, but I think we can come > up with something that we both like. We could make it so that the breakpoint/disabled breakpoint highlighting is used for the line number and the current line highlighting for the rest of the line. > I misunderstood the problem that you are having. Are you saying that > highlighting the line numbers with certain information isn't useful at > all times because you are scrolling to the right to the point where the > line numbers aren't even on the screen? No, the line numbers are displayed; my problem was simply that I find it difficult to visually match indented text all the way on the right side of the screen with the line number on the left side. Just knowing the current line number isn't much use, so I need a leader to see which line is selected. That's why I want to change the background color for the entire current line. > Would it be useful for CGDB to highlight lines with breakpoints by > highlighting the entire line red or yellow (based on breakpoint > enabled or disabled) instead of just the line number? Yes, that's another possibility. Let's say the breakpoints are highlighted by changing the foreground color, and the current line by changing the background, they could combine when coinciding. > Would it be useful for CGDB to highlight the current line the user is > browsing by highlighting the entire line instead of just the line > number? Yes, that's what my patch makes it do. :-) -- Haakon |
|
From: Bob R. <bob...@co...> - 2006-10-06 12:58:09
|
On Fri, Oct 06, 2006 at 01:38:50PM +0200, Haakon Riiser wrote: > > I misunderstood the problem that you are having. Are you saying that > > highlighting the line numbers with certain information isn't useful at > > all times because you are scrolling to the right to the point where the > > line numbers aren't even on the screen? > > No, the line numbers are displayed; my problem was simply that I > find it difficult to visually match indented text all the way on > the right side of the screen with the line number on the left side. > > Just knowing the current line number isn't much use, so I need a > leader to see which line is selected. That's why I want to change > the background color for the entire current line. I see, that seems reasonable. > > Would it be useful for CGDB to highlight the current line the user is > > browsing by highlighting the entire line instead of just the line > > number? > > Yes, that's what my patch makes it do. :-) OK. So simply highlighting the entire line you are on, and not the line number, would solve the problem for you, correct? Then the next question is, should I make this option treat all current functionality that highlights only the line number, highlight the entire line? I'll think about this. Steve, what is your opinion? Would this solve your problem? Mike, do you have an opinion on this? Thanks, Bob Rossi |
|
From: Haakon R. <haa...@fy...> - 2006-10-06 13:29:38
|
[Bob Rossi] > OK. So simply highlighting the entire line you are on, and not the line > number, would solve the problem for you, correct? Yes. -- Haakon |
|
From: Steve F. <st...@fo...> - 2006-10-06 19:52:10
|
On 10/6/06, Bob Rossi <bob...@co...> wrote: > Then the next question is, should I make this option treat all > current functionality that highlights only the line number, > highlight the entire line? I'll think about this. Steve, what is your > opinion? Would this solve your problem? Yes, assuming that a) it can be configured to use something other than "bold" to do the hilighting and b) it can be made to look good (on a black-on-white screen, of course!). And I'd prefer not to lose syntax hiliting on the current line, which means the attribute/background would need to be somehow *added* to the existing styles, instead of replacing them. |
|
From: Haakon R. <haa...@fy...> - 2006-10-06 20:18:56
|
[Steve Folta] > And I'd prefer not to lose syntax hiliting on the current line, > which means the attribute/background would need to be somehow > *added* to the existing styles, instead of replacing them. That shouldn't be problem, unless you use conflicting highlighting, of course. If the other syntax groups only set the foreground color, underline and bold attributes, and the current line is highlighted by setting the background color, they'll be able to stack without one rule replacing the other. -- Haakon |
|
From: Bob R. <bob...@co...> - 2006-10-06 20:35:45
|
On Fri, Oct 06, 2006 at 10:18:42PM +0200, Haakon Riiser wrote: > [Steve Folta] > > > And I'd prefer not to lose syntax hiliting on the current line, > > which means the attribute/background would need to be somehow > > *added* to the existing styles, instead of replacing them. > > That shouldn't be problem, unless you use conflicting highlighting, > of course. If the other syntax groups only set the foreground > color, underline and bold attributes, and the current line is > highlighted by setting the background color, they'll be able to > stack without one rule replacing the other. OK, All of these issues sound correct. If you don't mind, start CGDB on a program and go to the CGDB window and type ':set as=highlight'. This functionality should be extended to the current line being viewed, and the breakpoints. It should not cause the syntax highlighting information to be lost. Also, the highlighting colors should be configurable. Does this meet everyone's needs? If so, CGDB will need new options for breakpoints and the current line similar to the 'set as' option. It will also need new highlighting options that represnet how to highlight the lines when the corresponding options are on. I would be happy to accept a patch that meet this criteria, if everyone thinks this would be the correct approach. If I'm expected to do the work, it could be quite some time. Thanks, Bob Rossi |
|
From: Haakon R. <haa...@fy...> - 2006-10-06 21:18:51
|
[Bob Rossi] > OK, All of these issues sound correct. If you don't mind, start > CGDB on a program and go to the CGDB window and type > ':set as=highlight'. Already in my cgdbrc. :-) > This functionality should be extended to the current line being > viewed, and the breakpoints. It should not cause the syntax > highlighting information to be lost. Also, the highlighting colors > should be configurable. Does this meet everyone's needs? Sounds good to me. > I would be happy to accept a patch that meet this criteria, if > everyone thinks this would be the correct approach. If I'm > expected to do the work, it could be quite some time. I'll do it, once the requirements are nailed down. Until then, I'm content with the second patch I posted. -- Haakon |
