Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
Several improvements to Code Completion plugin
Huki:
--- Quote from: ollydbg on September 17, 2013, 07:22:13 am ---When begin, we have the index:
--- Code: ---/*// ... */
^
--- End code ---
If you run MoveToNextChar() first, you are now deliberately skip one character, and you are here in:
--- Code: ---/*// ... */
^
--- End code ---
Although this code did work correctly, but I think we can have a better method.
Method1: SkipToChar('*'); and see whether there is a '/' after '*'
Method2: if (PreviousChar() == '*' && CheckWeReallyMovedAfterSkipToChar)
I think method1 may be better, what's your opinion.
--- End quote ---
I don't think we need to change the method. A comment block always starts and ends with two chars, so deliberately skipping one char is a safe "shortcut" IMO. In the extreme case we will have:
Begin:
--- Code: ---/**/
^
--- End code ---
After running MoveToNextChar() first:
--- Code: ---/**/
^
--- End code ---
So it's going to be ok.
But if we really need more readable code, I suggest Method1 too.
Huki:
--- Quote from: ollydbg on September 17, 2013, 03:19:56 pm ---I do not understand what this code snippet trying to solve, can you explain?
--- Code: --- //FIX(huki), check whether m_TokenIndex has decreased which implies a ReplaceBufferForReparse() was done.
// We can also check for change in m_IsReplaceParsing and m_RepeatReplaceCount before and after DoGetToken().
if (m_IsReplaceParsing && savedTokenIndex > m_TokenIndex)
savedTokenIndex = m_TokenIndex;
--- End code ---
I have try to let the cctest project parsing a simple buffer:
With the replacement rule:
But I don't see the if(m_IsReplaceParsing && savedTokenIndex > m_TokenIndex) condition is true.
--- End quote ---
But with your test buffer, the replacement doesn't happen within PeekToken(). So of course, my fix won't be reached.
If we look at how PeekToken() works, we do the following:
--- Code: --- // 1) First we save existing token index:
unsigned int savedTokenIndex = m_TokenIndex;
unsigned int savedLineNumber = m_LineNumber;
unsigned int savedNestLevel = m_NestLevel;
// 2) Then we get new token. Within DoGetToken(), a ReplaceBufferForReparse() may occur,
// either to expand an actual macro, or because of user replacement token.
// This will rewind m_TokenIndex, so the savedTokenIndex above will become incorrect.
if (SkipUnwanted())
m_PeekToken = DoGetToken();
else
m_PeekToken.Clear();
// 3) Restore saved index. Because of above mentioned issue, this could end up restoring m_TokenIndex to wrong value.
// This will cause bad problems if UngetToken() is called after this PeekToken().
m_TokenIndex = savedTokenIndex;
m_LineNumber = savedLineNumber;
m_NestLevel = savedNestLevel;
--- End code ---
That is a PeekToken() specific fix. In general, we also need to reset m_UndoTokenIndex after a ReplaceBufferForReparse(). This is also done in the cc_fix_tokenizer.patch.
--- Code: ---Index: src/plugins/codecompletion/parser/tokenizer.cpp
===================================================================
--- src/plugins/codecompletion/parser/tokenizer.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/tokenizer.cpp (working copy)
@@ -1817,6 +1865,7 @@
// Fix token index
m_TokenIndex -= bufferLen;
+ m_UndoTokenIndex = m_TokenIndex; //ADD(huki)
// Update the peek token
if (m_PeekAvailable && updatePeekToken)
--- End code ---
oBFusCATed:
@huki: Can I ask you to provide a patch which adds you to the "help -> about -> thanks to" dialog?
ollydbg:
--- Quote from: Huki on September 17, 2013, 11:18:04 pm ---...
So it's going to be ok.
But if we really need more readable code, I suggest Method1 too.
--- End quote ---
Thanks for the comments on this, I have commit to SVN trunk now with "Method1". :)
--- Quote from: Huki on September 17, 2013, 11:01:45 pm ---...
For this subject, I suggest committing your ReadToEOL() patch and this DoAddToken() patch above.
--- End quote ---
This is also committed to SVN trunk now.
These are the commits I have done. (My time is UTC+8 based)
--- Quote ---Revision: 9343
Author: ollydbg
Date: 2013-9-18 9:59:45
Message:
* CC: when reading the parentheses, set the correct Tokenizer mode(tsReadRawExpression), also skip comments after the first '(' (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp
Revision: 9342
Author: ollydbg
Date: 2013-9-18 9:59:07
Message:
* CC: only skip after single equal sign, not double equals sign (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp
Revision: 9341
Author: ollydbg
Date: 2013-9-18 9:58:29
Message:
* CC: correctly find the end of a c style comment (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp
Revision: 9340
Author: ollydbg
Date: 2013-9-18 9:57:51
Message:
* CC: add missing breaks in switch case statements (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp
Revision: 9339
Author: ollydbg
Date: 2013-9-18 9:57:11
Message:
* CC: handle macro definition correctly, distinguish between function like macro definition and variable like definition (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/parserthread.cpp
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.h
--- End quote ---
Huki:
@ollydbg: Thanks! I see one fix related to '==' sign hasn't made it through, in ReadParantheses(). See the code below with my comment.
--- Code: --- case _T('='):
{
if (*(p - 1) <= _T(' '))
{
*p = _T('=');
// Don't add a space after '=' sign, in case another '=' follows it
// (see how the 'else' block below works).
//*++p = _T(' ');
++p;
}
else
{
switch (*(p - 1))
{
case _T('='):
case _T('!'):
case _T('>'):
case _T('<'):
{
*p = _T('=');
*++p = _T(' ');
++p;
}
break;
default:
{
*p = _T(' ');
*++p = _T('=');
*++p = _T(' ');
++p;
}
break;
}
}
}
break;
[...]
// '<', '>' and '!' are handled here.
default:
{
*p = ch;
++p;
}
break;
--- End code ---
And here is the patch:
--- Code: (Fixes space between '==') ---Index: src/plugins/codecompletion/parser/tokenizer.cpp
===================================================================
--- src/plugins/codecompletion/parser/tokenizer.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/tokenizer.cpp (working copy)
@@ -713,7 +718,6 @@
if (*(p - 1) <= _T(' '))
{
*p = _T('=');
- *++p = _T(' ');
++p;
}
else
--- End code ---
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version