Обсуждение: SQL Syntax Highlight Configuration
Hi there!
I'm tired to see always white background of SQL editor, while background color on my system has another color and is not so bright.
So I implemented feature to allow custom configuration for SQL syntax highlight, default background and foreground colors.
I hope you like it. See an attached patch and screenshot with options shown.
I've tested it on Windows Vista, hope it works on other platforms too.
Sincerely,
Maga Abdurakhmanov
Вложения
Hi Maga, Le 07/02/2010 02:43, Maga Abdurakhmanov a écrit : > [...] > I'm tired to see always white background of SQL editor, while background > color on my system has another color and is not so bright. > So I implemented feature to allow custom configuration for SQL syntax > highlight, default background and foreground colors. > > I hope you like it. See an attached patch and screenshot with options shown. > I've tested it on Windows Vista, hope it works on other platforms too. > I like it. Seems good to me. Works on Linux. I'm less sure for Mac OS X. Anyways, the UI needs some more work: * if "Use system background color" is checked, the associated wxColourPickerCtrl widgets should be disabled * the same for "Use system foreground color" * "Element colour x" doesn't mean anything to our users, the text should be replaced with a better text (for example "SQL words", "Text literals", or something better) When this is fixed, it'll be applied to the SVN (unless anyone objects). Thanks for working on this. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
I object. The correct solution here is to use the colour configured in the OS/window manager. When that is done, we'll also need a way to allow the user to configure the syntax highlighting colours so they don't clash. On 2/8/10, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Hi Maga, > > Le 07/02/2010 02:43, Maga Abdurakhmanov a écrit : >> [...] >> I'm tired to see always white background of SQL editor, while background >> color on my system has another color and is not so bright. >> So I implemented feature to allow custom configuration for SQL syntax >> highlight, default background and foreground colors. >> >> I hope you like it. See an attached patch and screenshot with options >> shown. >> I've tested it on Windows Vista, hope it works on other platforms too. >> > > I like it. Seems good to me. Works on Linux. I'm less sure for Mac OS X. > Anyways, the UI needs some more work: > > * if "Use system background color" is checked, the associated > wxColourPickerCtrl widgets should be disabled > * the same for "Use system foreground color" > * "Element colour x" doesn't mean anything to our users, the text > should be replaced with a better text (for example "SQL words", > "Text literals", or something better) > > When this is fixed, it'll be applied to the SVN (unless anyone objects). > > Thanks for working on this. > > > -- > Guillaume. > http://www.postgresqlfr.org > http://dalibo.com > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
I should read more carefully :-(. Ignore my last paragraph. On 2/8/10, Dave Page <dpage@pgadmin.org> wrote: > I object. > > The correct solution here is to use the colour configured in the > OS/window manager. > > When that is done, we'll also need a way to allow the user to > configure the syntax highlighting colours so they don't clash. > > On 2/8/10, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> Hi Maga, >> >> Le 07/02/2010 02:43, Maga Abdurakhmanov a écrit : >>> [...] >>> I'm tired to see always white background of SQL editor, while background >>> color on my system has another color and is not so bright. >>> So I implemented feature to allow custom configuration for SQL syntax >>> highlight, default background and foreground colors. >>> >>> I hope you like it. See an attached patch and screenshot with options >>> shown. >>> I've tested it on Windows Vista, hope it works on other platforms too. >>> >> >> I like it. Seems good to me. Works on Linux. I'm less sure for Mac OS X. >> Anyways, the UI needs some more work: >> >> * if "Use system background color" is checked, the associated >> wxColourPickerCtrl widgets should be disabled >> * the same for "Use system foreground color" >> * "Element colour x" doesn't mean anything to our users, the text >> should be replaced with a better text (for example "SQL words", >> "Text literals", or something better) >> >> When this is fixed, it'll be applied to the SVN (unless anyone objects). >> >> Thanks for working on this. >> >> >> -- >> Guillaume. >> http://www.postgresqlfr.org >> http://dalibo.com >> >> -- >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgadmin-hackers >> > > > -- > Dave Page > EnterpriseDB UK: http://www.enterprisedb.com > -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Le 08/02/2010 22:39, Dave Page a écrit : > I should read more carefully :-(. Ignore my last paragraph. > Sorry, I don't get it. You still have an objection to this patch or not ? -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Yes, I still object. The background colour should come from the OS or window manager as it would in other apps. The highlighting customisation is good though, and essential with the above. On 2/8/10, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Le 08/02/2010 22:39, Dave Page a écrit : >> I should read more carefully :-(. Ignore my last paragraph. >> > > Sorry, I don't get it. You still have an objection to this patch or not ? > > > -- > Guillaume. > http://www.postgresqlfr.org > http://dalibo.com > -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Le 08/02/2010 22:46, Dave Page a écrit : > Yes, I still object. The background colour should come from the OS or > window manager as it would in other apps. > > The highlighting customisation is good though, and essential with the above. > If I understand you correctly, Maga only has to get rid of the "Custom background" and "Custom foreground" widgets (and the code associated). -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
I don't understand the reason why I should get rid of custom background and foreground. With this patch you have an option to use system background colour, or if you like it you can use your own background color, the same is with the foreground color.
--
Sincerely,
Magomed
On Tue, Feb 9, 2010 at 12:50 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
Le 08/02/2010 22:46, Dave Page a écrit :> Yes, I still object. The background colour should come from the OS orIf I understand you correctly, Maga only has to get rid of the "Custom
> window manager as it would in other apps.
>
> The highlighting customisation is good though, and essential with the above.
>
background" and "Custom foreground" widgets (and the code associated).
--
--
Sincerely,
Magomed
I like it. Seems good to me. Works on Linux. I'm less sure for Mac OS X.
Did the system background color working on linux?
Anyways, the UI needs some more work:
* if "Use system background color" is checked, the associated
wxColourPickerCtrl widgets should be disabled
* the same for "Use system foreground color"
* "Element colour x" doesn't mean anything to our users, the text
should be replaced with a better text (for example "SQL words",
"Text literals", or something better)
When this is fixed, it'll be applied to the SVN (unless anyone objects).
Seems reasonable for me, i'll try to fix it.
Thanks for working on this.
--
Sincerely,
Magomed
Le 09/02/2010 07:17, Maga Abdurakhmanov a écrit : >> >> I like it. Seems good to me. Works on Linux. I'm less sure for Mac OS X. >> > > Did the system background color working on linux? > Yes. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Tue, Feb 9, 2010 at 6:14 AM, Maga Abdurakhmanov <maqdev@gmail.com> wrote: > I don't understand the reason why I should get rid of custom background and > foreground. With this patch you have an option to use system background > colour, or if you like it you can use your own background color, the same is > with the foreground color. I was going to say that basic elements of the user interface should follow the users configuration in their chosen theme or whatever is appropriate for their OS. We shouldn't start providing ways to override what the system is setup to do as doing so for standard elements essentially makes the themes mechanism redundant, and we'll potentially end up having to add colour selection for countless different things. That said, I just checked both Visual Studio and XCode, and both of those allow you to override the background colour as well as choose custom foreground highlighting colours, so I guess my argument is pretty weak :-( Therefore, objection retracted. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
* "Element colour x" doesn't mean anything to our users, the text
should be replaced with a better text (for example "SQL words",
"Text literals", or something better)
Anybody can help with this? I've found some description of elements in STC sources, but still have questions.
Do we really need SCE_SQL_SQLPLUS and SCE_SQL_SQLPLUS_PROMPT ? If do, then I need some appopriate description for configuration dialog.
Values 0 - 11 are used in pgadmin sources as a 1-12 colour elements relatively.
Within a bracket's i give my own description. Here is a full list of elements defined in SQL lexer for STC control:
#define SCE_SQL_DEFAULT 0 (I guess we should use just foregreound color (don't need to configure)
#define SCE_SQL_COMMENT 1 (multiline SQL comment)
#define SCE_SQL_COMMENTLINE 2 (single line SQL comment)
#define SCE_SQL_COMMENTDOC 3 (SQL doc within a comment)
#define SCE_SQL_NUMBER 4 (number)
#define SCE_SQL_WORD 5 (SQL keyword)
#define SCE_SQL_STRING 6 (double quoted string)
#define SCE_SQL_CHARACTER 7 (single quoted string)
#define SCE_SQL_SQLPLUS 8 (??)
#define SCE_SQL_SQLPLUS_PROMPT 9 (??)
#define SCE_SQL_OPERATOR 10 (operator)
#define SCE_SQL_IDENTIFIER 11 (identifier)
#define SCE_SQL_SQLPLUS_COMMENT 13
#define SCE_SQL_COMMENTLINEDOC 15
#define SCE_SQL_WORD2 16
#define SCE_SQL_COMMENTDOCKEYWORD 17
#define SCE_SQL_COMMENTDOCKEYWORDERROR 18
#define SCE_SQL_USER1 19
#define SCE_SQL_USER2 20
#define SCE_SQL_USER3 21
#define SCE_SQL_USER4 22
#define SCE_SQL_QUOTEDIDENTIFIER 23
Le 09/02/2010 22:48, Maga Abdurakhmanov a écrit : >> >> >> * "Element colour x" doesn't mean anything to our users, the text >> should be replaced with a better text (for example "SQL words", >> "Text literals", or something better) >> >> > Anybody can help with this? Not me at least. I don't know quite more about this. > I've found some description of elements in STC > sources, but still have questions. > > Do we really need SCE_SQL_SQLPLUS and SCE_SQL_SQLPLUS_PROMPT ? If do, then I > need some appopriate description for configuration dialog. > They can be used, so yes, we need them. Perhaps Dave can tell us more about them ? > Values 0 - 11 are used in pgadmin sources as a 1-12 colour elements > relatively. > Within a bracket's i give my own description. Here is a full list of > elements defined in SQL lexer for STC control: > > #define SCE_SQL_DEFAULT 0 (I guess we should use just foregreound color > (don't need to configure) > #define SCE_SQL_COMMENT 1 (multiline SQL comment) > #define SCE_SQL_COMMENTLINE 2 (single line SQL comment) > #define SCE_SQL_COMMENTDOC 3 (SQL doc within a comment) > #define SCE_SQL_NUMBER 4 (number) > #define SCE_SQL_WORD 5 (SQL keyword) > #define SCE_SQL_STRING 6 (double quoted string) > #define SCE_SQL_CHARACTER 7 (single quoted string) > #define SCE_SQL_SQLPLUS 8 (??) > #define SCE_SQL_SQLPLUS_PROMPT 9 (??) > #define SCE_SQL_OPERATOR 10 (operator) > #define SCE_SQL_IDENTIFIER 11 (identifier) > > #define SCE_SQL_SQLPLUS_COMMENT 13 > #define SCE_SQL_COMMENTLINEDOC 15 > #define SCE_SQL_WORD2 16 > #define SCE_SQL_COMMENTDOCKEYWORD 17 > #define SCE_SQL_COMMENTDOCKEYWORDERROR 18 > #define SCE_SQL_USER1 19 > #define SCE_SQL_USER2 20 > #define SCE_SQL_USER3 21 > #define SCE_SQL_USER4 22 > #define SCE_SQL_QUOTEDIDENTIFIER 23 > AFAIR, we only use from 0 to 11. And apart from 8 and 9, all are simple to understand. If we don't have a better answer from Dave, we could probably drop them (use the old value, and don't customize it). -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Hi, I've made changes to the patch according to Guillaume suggestions.
See an attached file. Thanks!
--
Sincerely,
Magomed
* if "Use system background color" is checked, the associated
wxColourPickerCtrl widgets should be disabled
* the same for "Use system foreground color"
* "Element colour x" doesn't mean anything to our users, the text
should be replaced with a better text (for example "SQL words",
"Text literals", or something better)
When this is fixed, it'll be applied to the SVN (unless anyone objects).
Thanks for working on this.
--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com
--
Sincerely,
Magomed
Вложения
Le 13/02/2010 20:04, Maga Abdurakhmanov a écrit : > Hi, I've made changes to the patch according to Guillaume suggestions. > See an attached file. Thanks! > So far, so good. I only have one complaint. The UI on GTK is weird. Did you work on Windows? (the "Stripping trailing CRs from patch." messages seem to indicate that) On GTK, the text is too big. See attached screenshot. If you can't work on this on Linux/GTK, I'll do. Just tell me. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Вложения
Le 13/02/2010 22:30, Guillaume Lelarge a écrit : > Le 13/02/2010 20:04, Maga Abdurakhmanov a écrit : >> Hi, I've made changes to the patch according to Guillaume suggestions. >> See an attached file. Thanks! >> > > So far, so good. > > I only have one complaint. The UI on GTK is weird. Did you work on > Windows? (the "Stripping trailing CRs from patch." messages seem to > indicate that) > > On GTK, the text is too big. See attached screenshot. > > If you can't work on this on Linux/GTK, I'll do. Just tell me. > I answer to myself. We just need to change the width of the colourpicker control (from 50 to 25). Can you try on your system if it works great this way? I see no other objection. Dave, are you still OK that I apply this? -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Sat, Feb 13, 2010 at 9:46 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Le 13/02/2010 22:30, Guillaume Lelarge a écrit : >> Le 13/02/2010 20:04, Maga Abdurakhmanov a écrit : >>> Hi, I've made changes to the patch according to Guillaume suggestions. >>> See an attached file. Thanks! >>> >> >> So far, so good. >> >> I only have one complaint. The UI on GTK is weird. Did you work on >> Windows? (the "Stripping trailing CRs from patch." messages seem to >> indicate that) >> >> On GTK, the text is too big. See attached screenshot. >> >> If you can't work on this on Linux/GTK, I'll do. Just tell me. >> > > I answer to myself. We just need to change the width of the colourpicker > control (from 50 to 25). > > Can you try on your system if it works great this way? > > I see no other objection. Dave, are you still OK that I apply this? Go ahead. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Le 13/02/2010 22:51, Dave Page a écrit : > On Sat, Feb 13, 2010 at 9:46 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> Le 13/02/2010 22:30, Guillaume Lelarge a écrit : >>> Le 13/02/2010 20:04, Maga Abdurakhmanov a écrit : >>>> Hi, I've made changes to the patch according to Guillaume suggestions. >>>> See an attached file. Thanks! >>>> >>> >>> So far, so good. >>> >>> I only have one complaint. The UI on GTK is weird. Did you work on >>> Windows? (the "Stripping trailing CRs from patch." messages seem to >>> indicate that) >>> >>> On GTK, the text is too big. See attached screenshot. >>> >>> If you can't work on this on Linux/GTK, I'll do. Just tell me. >>> >> >> I answer to myself. We just need to change the width of the colourpicker >> control (from 50 to 25). >> >> Can you try on your system if it works great this way? >> >> I see no other objection. Dave, are you still OK that I apply this? > > Go ahead. > > Thanks. I'm waiting on Maga's answer about the change of width, but will do afterwards. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Hi!
Seems like that size parameter of color picker is ignored on Windows. I've changed size of color pickers to <size>25,12d</size> and nothing is changed (see an attach). On windows all texts fits fine. I can't test it on GTK/Linux, sorry.
Thanks. I'm waiting on Maga's answer about the change of width, but will
do afterwards.
--
Sincerely,
Magomed
Вложения
Le 14/02/2010 10:44, Maga Abdurakhmanov a écrit : > Hi! > > Seems like that size parameter of color picker is ignored on Windows. I've > changed size of color pickers to <size>25,12d</size> and nothing is changed > (see an attach). On windows all texts fits fine. I can't test it on > GTK/Linux, sorry. > I checked on GTK and it worked. Thanks for checking on Windows. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Le 14/02/2010 10:58, Guillaume Lelarge a écrit : > Le 14/02/2010 10:44, Maga Abdurakhmanov a écrit : >> Hi! >> >> Seems like that size parameter of color picker is ignored on Windows. I've >> changed size of color pickers to <size>25,12d</size> and nothing is changed >> (see an attach). On windows all texts fits fine. I can't test it on >> GTK/Linux, sorry. >> > > I checked on GTK and it worked. > > Thanks for checking on Windows. > Thanks for your work. I've commited your patch. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Hi!
I've found a bug in my previous patch. Font size when highlighting braces is different from default.
Please see fix in attach. Thanks!
Вложения
Any chance to apply this fix?
--
Sincerely,
Magomed
On Tue, Feb 23, 2010 at 4:51 PM, Maga Abdurakhmanov <maqdev@gmail.com> wrote:
Hi!I've found a bug in my previous patch. Font size when highlighting braces is different from default.Please see fix in attach. Thanks!
--
Sincerely,
Magomed
Вложения
Le 03/03/2010 22:02, Maga Abdurakhmanov a écrit : > Any chance to apply this fix? > My bad. I completely forgot about it when I was on holidays. Thanks for reminding me. Let me check this first. > On Tue, Feb 23, 2010 at 4:51 PM, Maga Abdurakhmanov <maqdev@gmail.com>wrote: > >> Hi! >> I've found a bug in my previous patch. Font size when highlighting braces >> is different from default. >> Please see fix in attach. Thanks! >> > -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Le 03/03/2010 22:04, Guillaume Lelarge a écrit : > Le 03/03/2010 22:02, Maga Abdurakhmanov a écrit : >> Any chance to apply this fix? >> > > My bad. I completely forgot about it when I was on holidays. Thanks for > reminding me. Let me check this first. > Applied, thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com