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