It looks like I had installed codeblocks with libwxgtk3.0 unofficial. Under that setup, the code ran fine. But when I changed to libwxgtk2.8, I saw an error about the "_T()" you had pointed out. After fixing it, the code performs correctly.
Did you try running the code with the "_T()" corrected? If not, I've attached a corrected version.
Hi, jat1, thanks.
So, the rev2 only have _T() issue fixed against rev1?
I have already fixed the _T() issue in my local copy. The actual problem is your patch (either rev1 or rev2) don't fix the problem I described in Link.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yes, rev2 only fixed the _T() issue. When I try the code you posted above, this happens:
If I type "exp" then CC shows "exp123"
If I type "exp123." then CC shows "a"
This is the behavior we want, correct? I've been using the attached test suite. I haven't been able to get cctest working yet. But once I do, I'll post its results as well.
I find that when we try to parse the "E exp123" in the function void ParserThread::HandleConditionalArguments(), we first get a token "E", then we reached the end of the buffer, and the function call wxString peek = smallTokenizer.PeekToken(); get an empty token, so, this way, we can't add the "exp123" to the tokentree.
I think we need to add more chars in bool Tokenizer::InitFromBuffer() function, or maybe, we need more spaces after "E exp123". In this case, we don't need to remove the "(" and ")" in the code:
// remove braces
if (args.StartsWith(_T("(")))
args = args.Mid(1, args.length() - 1);
if (args.EndsWith(_T(")")))
args = args.Mid(0, args.length() - 1);
We can just replace them with spaces.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yeah, that was the same reasoning behind my adding the equals symbol. But changing the parenthesis to spaces is more elegant. The patch passes my tests, so I'm glad we arrived at something that works for both of us.
Also, looking at your code, I think I figured out why my first/second patch wasn't working for you. In my patches, I put my new code in ParserThread::HandleConditionalArguments(). But in your code, my changes were added to ParserThread::HandleForLoopArguments().
Anyway, I cleaned up some parts of your latest code, mainly taking out old lines that were no longer needed. Also, since this patch overlaps with code for other conditional statements, I checked that those still worked as well. I've attached these changes. Thanks for you help, I think this revision is the best.
Hi, jat1, sorry for late reply, I totally agree with your analysis in the previous post, and the correct fix should in ParserThread::HandleConditionalArguments() not ParserThread::HandleForLoopArguments(). I think the reason the patch goes to wrong function body is that I have some other part of code changes in parserthread.cpp locally, so the patch utility just run some fuzzy guess when applying your patch. Note that those two functions have some common code...Oh, maybe, we may need a code refactoring about those two functions......
I have applied your rev3 patch in the trunk now. Many thanks for your contribution!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi, I've attached a patch for this bug.
It looks like your patch don't works OK with the following test code:
But it doesn't prompt the "exp123".
By the way: It should have a _T() in the statement: args.Append(_T("=0"));
It looks like I had installed codeblocks with libwxgtk3.0 unofficial. Under that setup, the code ran fine. But when I changed to libwxgtk2.8, I saw an error about the "_T()" you had pointed out. After fixing it, the code performs correctly.
Did you try running the code with the "_T()" corrected? If not, I've attached a corrected version.
Hi, jat1, thanks.
So, the rev2 only have _T() issue fixed against rev1?
I have already fixed the _T() issue in my local copy. The actual problem is your patch (either rev1 or rev2) don't fix the problem I described in Link.
Hi ollydbg,
Yes, rev2 only fixed the _T() issue. When I try the code you posted above, this happens:
If I type "exp" then CC shows "exp123"
If I type "exp123." then CC shows "a"
This is the behavior we want, correct? I've been using the attached test suite. I haven't been able to get cctest working yet. But once I do, I'll post its results as well.
Hi, thanks, I just retest the patch again, but in my test here, I don't show "exp123", that's really strange. See the screen shot in attachments.
I find that when we try to parse the "E exp123" in the function void ParserThread::HandleConditionalArguments(), we first get a token "E", then we reached the end of the buffer, and the function call wxString peek = smallTokenizer.PeekToken(); get an empty token, so, this way, we can't add the "exp123" to the tokentree.
I think we need to add more chars in bool Tokenizer::InitFromBuffer() function, or maybe, we need more spaces after "E exp123". In this case, we don't need to remove the "(" and ")" in the code:
We can just replace them with spaces.
This patch should work OK, see attachment below.
Last edit: ollydbg 2015-04-19
Hi ollydbg,
Yeah, that was the same reasoning behind my adding the equals symbol. But changing the parenthesis to spaces is more elegant. The patch passes my tests, so I'm glad we arrived at something that works for both of us.
Also, looking at your code, I think I figured out why my first/second patch wasn't working for you. In my patches, I put my new code in ParserThread::HandleConditionalArguments(). But in your code, my changes were added to ParserThread::HandleForLoopArguments().
Anyway, I cleaned up some parts of your latest code, mainly taking out old lines that were no longer needed. Also, since this patch overlaps with code for other conditional statements, I checked that those still worked as well. I've attached these changes. Thanks for you help, I think this revision is the best.
Hi, jat1, sorry for late reply, I totally agree with your analysis in the previous post, and the correct fix should in ParserThread::HandleConditionalArguments() not ParserThread::HandleForLoopArguments(). I think the reason the patch goes to wrong function body is that I have some other part of code changes in parserthread.cpp locally, so the patch utility just run some fuzzy guess when applying your patch. Note that those two functions have some common code...Oh, maybe, we may need a code refactoring about those two functions......
I have applied your rev3 patch in the trunk now. Many thanks for your contribution!
I reopen this ticket, since Huki has some report, see:
CC: [huki] parser - reverted part of commit r10230 to fix regression …
I have an explanation why r10230 causes regression, see details here: Re: Check macro usage on every identifier like token: issues and discussions
revision 10557 revert some changes, so the regression is fixed.
Strange, I still see this issue in version 20.03.