Hi guys,
My name is Shankhoneer Chakrovarty (aka 'shankhs' ), I have been using geany for past 3-4 years (mainly on my ubuntu ) and I have learned all my programming skills on geany. So thank you!
Since I have been programming for 3-4 years now and also using geany, I thought it will be great if I could give something back to the geany community.
I pulled the geany source code, read the HACKING file and created a patch which categorizes the compiler errors into "error" and "warning" and correspondingly draws a different colored squiggle underline in the line which caused the error. I have created the patch as mentioned in the hacking file and attached with this mail. If you guys have some time, can you please review the patch? If you need any more info, please feel free to drop me an email, I will be more than happy to explain it.
Also, I am reading the geany source code and modified some part of it to suit my personal needs so if you need a hand in resolving bugs, please do tell me. I am particularly interested in https://sourceforge.net/p/geany/bugs/254/ .
Thanks, Shankhoneer Chakrivarty
On 11 March 2014 19:35, shan chak shankholove@gmail.com wrote:
Hi guys,
My name is Shankhoneer Chakrovarty (aka 'shankhs' ), I have been using geany for past 3-4 years (mainly on my ubuntu ) and I have learned all my programming skills on geany. So thank you!
Since I have been programming for 3-4 years now and also using geany, I thought it will be great if I could give something back to the geany community.
I pulled the geany source code, read the HACKING file and created a patch which categorizes the compiler errors into "error" and "warning" and correspondingly draws a different colored squiggle underline in the line which caused the error. I have created the patch as mentioned in the hacking file and attached with this mail. If you guys have some time, can you please review the patch? If you need any more info, please feel free to drop me an email, I will be more than happy to explain it.
Have not reviewed it in detail, but some comments:
1. don't change whitespace (eg blank lines, that just makes noise in the patch that is not needed making real review harder) 2. don't use doxygen comments for in-code comments 3. watch the layout, you are missing many spaces after commas 4. is setting a pointer to a piece of the string and comparing it to "warning" really the best way of returning that information rather than a bool or enum? 5. compiler lines are also parsed by filetypes_parse_compiler_line()
The preferred way of providing changes is now via pull requests on github rather than patches. The HACKING needs updating.
Cheers Lex
Also, I am reading the geany source code and modified some part of it to suit my personal needs so if you need a hand in resolving bugs, please do tell me. I am particularly interested in https://sourceforge.net/p/geany/bugs/254/ .
See also #907 :-D
Thanks, Shankhoneer Chakrivarty
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Hi !
I try to update the HACKING file considering the need. Feel free to comment and improve :)
https://github.com/geany/geany/pull/224
See you !
2014-03-11 10:54 GMT+01:00 Lex Trotman elextr@gmail.com:
On 11 March 2014 19:35, shan chak shankholove@gmail.com wrote:
Hi guys,
My name is Shankhoneer Chakrovarty (aka 'shankhs' ), I have been using
geany
for past 3-4 years (mainly on my ubuntu ) and I have learned all my programming skills on geany. So thank you!
Since I have been programming for 3-4 years now and also using geany, I thought it will be great if I could give something back to the geany community.
I pulled the geany source code, read the HACKING file and created a patch which categorizes the compiler errors into "error" and "warning" and correspondingly draws a different colored squiggle underline in the line which caused the error. I have created the patch as mentioned in the
hacking
file and attached with this mail. If you guys have some time, can you please review the patch? If you need
any
more info, please feel free to drop me an email, I will be more than
happy
to explain it.
Have not reviewed it in detail, but some comments:
- don't change whitespace (eg blank lines, that just makes noise in
the patch that is not needed making real review harder) 2. don't use doxygen comments for in-code comments 3. watch the layout, you are missing many spaces after commas 4. is setting a pointer to a piece of the string and comparing it to "warning" really the best way of returning that information rather than a bool or enum? 5. compiler lines are also parsed by filetypes_parse_compiler_line()
The preferred way of providing changes is now via pull requests on github rather than patches. The HACKING needs updating.
Cheers Lex
Also, I am reading the geany source code and modified some part of it to suit my personal needs so if you need a hand in resolving bugs, please do tell me. I am particularly interested in https://sourceforge.net/p/geany/bugs/254/ .
See also #907 :-D
Thanks, Shankhoneer Chakrivarty
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Thanks Lex. Please see my comments and questions inline.
On Tue, Mar 11, 2014 at 2:54 AM, Lex Trotman elextr@gmail.com wrote:
On 11 March 2014 19:35, shan chak shankholove@gmail.com wrote:
Hi guys,
My name is Shankhoneer Chakrovarty (aka 'shankhs' ), I have been using
geany
for past 3-4 years (mainly on my ubuntu ) and I have learned all my programming skills on geany. So thank you!
Since I have been programming for 3-4 years now and also using geany, I thought it will be great if I could give something back to the geany community.
I pulled the geany source code, read the HACKING file and created a patch which categorizes the compiler errors into "error" and "warning" and correspondingly draws a different colored squiggle underline in the line which caused the error. I have created the patch as mentioned in the
hacking
file and attached with this mail. If you guys have some time, can you please review the patch? If you need
any
more info, please feel free to drop me an email, I will be more than
happy
to explain it.
Have not reviewed it in detail, but some comments:
- don't change whitespace (eg blank lines, that just makes noise in
the patch that is not needed making real review harder)
[Shankhoneer] Point noted. I will remove these noises in my patch
2. don't use doxygen comments for in-code comments
[Shankhoneer] I have changed all in-code doxygen comments to c-style comments
- watch the layout, you are missing many spaces after commas
[Shankhoneer] Fixed this too
- is setting a pointer to a piece of the string and comparing it to
"warning" really the best way of returning that information rather than a bool or enum?
[Shankhoneer] I dont understand exactly what you are trying to suggest. Do you mean that I should create a function that returns which color the line should be given the error message? I have never used C before in production software as big as geany so please feel free to comment directly what would be a better approach.
- compiler lines are also parsed by filetypes_parse_compiler_line()
[Shankhoneer] I couldnt find filetypes_parse_compiler_line() but I found filetypes_parse_error_message() in filetypes.c
The preferred way of providing changes is now via pull requests on github rather than patches. The HACKING needs updating.
[Shankhoneer] I have forked the project and working on the forked repo.
Also, I am reading the geany source code and modified some part of it to suit my personal needs so if you need a hand in resolving bugs, please do tell me. I am particularly interested in https://sourceforge.net/p/geany/bugs/254/ .
See also #907 :-D
[Shankhoneer] Woah! #907 is like a long TODO list for C++11. I have started to look into some of the issues like 5e.
Thanks, Shankhoneer Chakrovarty
[...]
- is setting a pointer to a piece of the string and comparing it to
"warning" really the best way of returning that information rather than a bool or enum?
[Shankhoneer] I dont understand exactly what you are trying to suggest. Do you mean that I should create a function that returns which color the line should be given the error message? I have never used C before in production software as big as geany so please feel free to comment directly what would be a better approach.
You seemed to be returning "is this a warning or not", so thats a boolean condition that can be tested locally and returned as such. Returning a pointer to part of the message and then testing it in code far far away is likely to be confusing for maintenance.
- compiler lines are also parsed by filetypes_parse_compiler_line()
[Shankhoneer] I couldnt find filetypes_parse_compiler_line() but I found filetypes_parse_error_message() in filetypes.c
er, yeah, that one, stupid fingers autotyping :)
The preferred way of providing changes is now via pull requests on github rather than patches. The HACKING needs updating.
[Shankhoneer] I have forked the project and working on the forked repo.
Then it would be best to make a pull request on github so we can attach comments to the source, thanks.
Also, I am reading the geany source code and modified some part of it to suit my personal needs so if you need a hand in resolving bugs, please do tell me. I am particularly interested in https://sourceforge.net/p/geany/bugs/254/ .
See also #907 :-D
[Shankhoneer] Woah! #907 is like a long TODO list for C++11. I have started to look into some of the issues like 5e.
WARNING: many of those are still in the list because they are not easy :)
5 e? sigh, stupid sourceforge, its new version has autonumbered an old manually numbered list, what do they expect, that everyone should be going through all their bugs and editing them to suit the new style? :(
Anyway 5 e is probably a Scintilla issue, which is a separate project which we use, www.scintilla.org. You should send patches there so its confirmed that the change is acceptable to them.
Cheers Lex
Thanks, Shankhoneer Chakrovarty
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
[...]
Anyway 5 e is probably a Scintilla issue, which is a separate project which we use, www.scintilla.org. You should send patches there so its confirmed that the change is acceptable to them.
Update, its already fixed in the current version of Scintilla.
Cheers Lex
Cheers Lex
Thanks, Shankhoneer Chakrovarty
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Thanks Lex. Sorry for the late reply.
You seemed to be returning "is this a warning or not", so thats a
boolean condition that can be tested locally and returned as such. Returning a pointer to part of the message and then testing it in code far far away is likely to be confusing for maintenance.
I agree and I understand your concern. The problem I have is if I modify the functions (msgwin_parse_compiler_error_line and others in the chain) to return a boolean value(whether its an error or warning) I have no way to test whether the function continues to work for all the filetypes supported by geany. Functions like msgwin_parse_compiler_error_line and others are extremely generic and is supposed to work for all the filetypes. I have tested my code for C, C++, Java, Python and LaTex and it seems to work. This brings me to my second point, can you please share with me any testing code that you guys might have?
Then it would be best to make a pull request on github so we can attach comments to the source, thanks.
I have made the pull request. Again thank you for being so patient and helping me understand all the nitty-gritties of geany source code.
Thanks, Shankhoneer Chakrovarty
On Tue, 11 Mar 2014 01:35:50 -0700 shan chak shankholove@gmail.com wrote:
I pulled the geany source code, read the HACKING file and created a patch which categorizes the compiler errors into "error" and "warning" [...]
If you guys have some time, can you please review the patch? If you need any more info, please feel free to drop me an email, I will be more than happy to explain it.
- data.min_fields = 3; + data.min_fields = 5;
Please note that parse_file_line() returns if length(fields) is less than data->min_fields. With the above change, anything with < 4 colons will be rejected.
+ if (g_strv_length(fields) == data->min_fields)
N/A: parse_file_line() returns on length(fields) < min_fields, and the maximum fields to parse are min_fields, so length(fields) is always equal to min_fields at this point.
+ { + *type = g_strstrip(g_strdup(fields[data->line_idx+2]));
Errr, why do you assume that type is always at line_idx+2?..
--
Finally:
1. You wrote support for the fallback parser only, and not for regex.
2. This should be considered after either pull request #191 or SF patch #11 is applied. If the PR is chosen, this patch should be fully rewritten as capturing group <W> or something similar.
Thanks Dimitar.
On Thu, Mar 13, 2014 at 11:42 AM, Dimitar Zhekov dimitar.zhekov@gmail.comwrote:
- data.min_fields = 3;
- data.min_fields = 5;
Please note that parse_file_line() returns if length(fields) is less than data->min_fields. With the above change, anything with < 4 colons will be rejected.
Thanks for pointing out the error. I am now extracting all the tokens and then comparing if number of tokens I have is less than data->min_fields.
- if (g_strv_length(fields) == data->min_fields)
N/A: parse_file_line() returns on length(fields) < min_fields, and the maximum fields to parse are min_fields, so length(fields) is always equal to min_fields at this point.
Thanks for pointing that out, I have made the corrections
+ {
- *type = g_strstrip(g_strdup(fields[data->line_idx+2]));
Errr, why do you assume that type is always at line_idx+2?..
As far as I could understand, geany parses the error message by gcc compiler. gcc compilers have specific error message format ( http://gcc.gnu.org/onlinedocs/gcc-4.5.0/gnat_ugn_unw/Warning-Message-Control...) which is of the form <filename>:<line number>:<column number>:<warning.error>:<message>. This format guarantees that 2 fields after <line number> will be <warning/error>. So I used line_idx+2 to denote type_idx. I should declare type_idx variable and assign line_idx+2 to it which I think is more maintainable than the current version. What do you think?
--
Finally:
- You wrote support for the fallback parser only, and not for regex.
Yes because the regex parser was returning me NULL for c,cpp and java files, so I wrote for the fallback parser only. I was unable to find the root cause of why the regex parser is returning me NULL for these filetypes.
- This should be considered after either pull request #191 or SF
patch #11 is applied. If the PR is chosen, this patch should be fully rewritten as capturing group <W> or something similar.
Thanks,
Shankhoneer Chakrovarty
On 3/16/14, Shankhoneer Chakrovarty shankhoneer@gmail.com wrote:
On Thu, Mar 13, 2014 at 11:42 AM, Dimitar Zhekov dimitar.zhekov@gmail.comwrote:
- {
- *type = g_strstrip(g_strdup(fields[data->line_idx+2]));
Errr, why do you assume that type is always at line_idx+2?..
As far as I could understand, geany parses the error message by gcc compiler. gcc compilers have specific error message format [...]
First, there are several build-in parser configurations, with different separators and field indexes, example messages included. D and HTML also have warnings.
Second, the examples under "All GNU gcc-like error messages" have only "filename:line:whatever" is common, and the parser matches that format. Nowhere is a column assumed, and line_idx+2 may not even exist. The java examples clearly have type at line_idx + 1.
http://gcc.gnu.org/onlinedocs/gcc-4.5.0/gnat_ugn_unw/Warning-Message-Control...
This is for the gcc Ada compiler, and even if the entire gcc standartized is standartized at 4.5, the stable debian still contains gcc-4.4, so it's too early to drop it. And anyway, Latex will not gain more fields because the gcc documentation says so...
- You wrote support for the fallback parser only, and not for regex.
Yes because the regex parser was returning me NULL for c,cpp and java files, so I wrote for the fallback parser only. I was unable to find the root cause of why the regex parser is returning me NULL for these file types.
Well, I can assure you that the regex parser works...
- This should be considered after either pull request #191 or SF
patch #11 is applied. If the PR is chosen, this patch should be fully rewritten as capturing group <W> or something similar.
...*sigh* maybe I should include warning recognition is SF #11, using your styling code and different recognition.
-- E-gards: Jimmy
Thanks Dimitar. Please find my reply below.
On Thu, Mar 13, 2014 at 11:42 AM, Dimitar Zhekov dimitar.zhekov@gmail.comwrote:
- {
- *type = g_strstrip(g_strdup(fields[data->line_idx+2]));
Errr, why do you assume that type is always at line_idx+2?..
As far as I could understand, geany parses the error message by gcc compiler. gcc compilers have specific error message format [...]
First, there are several build-in parser configurations, with different separators and field indexes, example messages included. D and HTML also have warnings.
Second, the examples under "All GNU gcc-like error messages" have only "filename:line:whatever" is common, and the parser matches that format. Nowhere is a column assumed, and line_idx+2 may not even exist. The java examples clearly have type at line_idx + 1.
http://gcc.gnu.org/onlinedocs/gcc-4.5.0/gnat_ugn_unw/Warning-Message-Control...
This is for the gcc Ada compiler, and even if the entire gcc standartized is standartized at 4.5, the stable debian still contains gcc-4.4, so it's too early to drop it. And anyway, Latex will not gain more fields because the gcc documentation says so...
Thanks Dimitar for clearing things up. I know the error messages do not have standard format and more so it varies a lot from one programming language to another. Also, not every programming language has two types of error messages(warning and error) like C/C++. What I wrote is specific to C/C++ and if the error message has a different format it falls back to the behavior geany currently has.
- You wrote support for the fallback parser only, and not for regex.
Yes because the regex parser was returning me NULL for c,cpp and java files, so I wrote for the fallback parser only. I was unable to find the root cause of why the regex parser is returning me NULL for these file types.
Well, I can assure you that the regex parser works...
Ok. Let me try again to understand whats going on.
- This should be considered after either pull request #191 or SF
patch #11 is applied. If the PR is chosen, this patch should be fully rewritten as capturing group <W> or something similar.
...*sigh* maybe I should include warning recognition is SF #11, using your styling code and different recognition.
Can you please tell me what is SF and PR? Also, can you please assign me a bug or a feature request? I know I can pick anything from the list but since I am new to this project, I think it will be better for me not to chose by myself any bug/feature thats too complicated for me to implement and mess up the code.
Thanks, Shankhoneer Chakrovarty
On Tue, 18 Mar 2014 09:26:54 -0700 Shankhoneer Chakrovarty shankhoneer@gmail.com wrote:
[...]
Can you please tell me what is SF and PR?
Local jargon. :) SF bug/patch # = Geany sourceforge bug/patch tracker item #. PR # = Geany githut pull request #.
Also, can you please assign me a bug or a feature request? I know I can pick anything from the list but since I am new to this project, I think it will be better for me not to chose by myself any bug/feature thats too complicated for me to implement and mess up the code.
Well, an uncomplicated non-fixed bug is not easy to find, and the leading developers should have a better look than me. You can try http://sourceforge.net/p/geany/bugs/839/ if you have Windows.
As of the feature requests, they are highly subjective - everyone wants his or her own thing, which nobody else may need/like. So you can just choose whatever seems useful to you.
On 14-03-18 09:26 AM, Shankhoneer Chakrovarty wrote:
[...] Can you please tell me what is SF and PR? Also, can you please assign me a bug or a feature request? I know I can pick anything from the list but since I am new to this project, I think it will be better for me not to chose by myself any bug/feature thats too complicated for me to implement and mess up the code.
You can also check the sister project Geany-Plugins which is likely to need more love than Geany core. If I remember correctly there are at least several plugins without maintainers, and we are also trying to get all plugins to build with GTK3, as well as probably loads of bugs and feature requests on the bug tracker (linked from geany.org bugs page).
Cheers, Matthew Brush
Thanks Matthew and Dimitar.
Can you please tell me what is SF and PR? Also, can you please assign me a
bug or a feature request? I know I can pick anything from the list but since I am new to this project, I think it will be better for me not to chose by myself any bug/feature thats too complicated for me to implement and mess up the code.
You can also check the sister project Geany-Plugins which is likely to need more love than Geany core. If I remember correctly there are at least several plugins without maintainers, and we are also trying to get all plugins to build with GTK3, as well as probably loads of bugs and feature requests on the bug tracker (linked from geany.org bugs page).
Can you please tell me one such plugin? There are so many plugins that its very tough to get an idea which plugins are still alive and has a maintainer and which not.
Thanks, Shankhoneer Chakrovarty
On 14-03-18 03:28 PM, Shankhoneer Chakrovarty wrote:
Thanks Matthew and Dimitar.
Can you please tell me what is SF and PR? Also, can you please assign me a
bug or a feature request? I know I can pick anything from the list but since I am new to this project, I think it will be better for me not to chose by myself any bug/feature thats too complicated for me to implement and mess up the code.
You can also check the sister project Geany-Plugins which is likely to need more love than Geany core. If I remember correctly there are at least several plugins without maintainers, and we are also trying to get all plugins to build with GTK3, as well as probably loads of bugs and feature requests on the bug tracker (linked from geany.org bugs page).
Can you please tell me one such plugin? There are so many plugins that its very tough to get an idea which plugins are still alive and has a maintainer and which not.
In the MAINTAINERS file, there's a field 'S' (for status), any with 'Orphan' are for sure orphaned, others may be as well, but just not officially or updated in that file.
If you meant for porting to GTK3, I'm not sure if there's an official list of unported ones, but it should be reasonably simple to look inside the build/*.m4 file for the plugin to see if it has GTK3 build system support yet.
Cheers, Matthew Brush
Am 18.03.2014 23:48, schrieb Matthew Brush:
In the MAINTAINERS file, there's a field 'S' (for status), any with 'Orphan' are for sure orphaned, others may be as well, but just not officially or updated in that file.
Yes.
If you meant for porting to GTK3, I'm not sure if there's an official list of unported ones, but it should be reasonably simple to look inside the build/*.m4 file for the plugin to see if it has GTK3 build system support yet.
There isn't any. Even the "portend" ones are lagging a lot of testing, so even they can be compiled with Gtk3, they might fail at some point. Unfortunately.
Cheers, Frank