@kugel- to be clear, nothing I said is a general criticism of the purpose your 
PR or the implementation (apart from the specific comments you have already 
fixed).  

I just found it difficult to understand it, not aided by lower case macros that 
"capture" following syntax.  Also as you pointed out above, not only is it 
lower case, it is an unprefixed Geany local macro.  So I was not confident I 
understood the implementation, and couldn't justify spending more time on it.  
Thats why I approved my review, so someone else could review it and not need to 
override my review.

Yes I believe macros like foreach_strv and foreach_document etc should be 
removed, but thats not part of this PR, will open another issue for discussions 
about that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1445#issuecomment-289007035

Reply via email to