Skip to content

Commit

Permalink
Fix several bugs:
Browse files Browse the repository at this point in the history 
1. Lexer. When spaces exist between ";" and "/", it is still treated as multi-line comment.
2. Keyword matcher. When nested flow control is open-ended, it causes infinite recursion.
3. Keyword matcher. Keyword in string and comment can trigger keyword matching.
4. Keyword matcher. Keyword in string can be incorrectly picked as matching keyword.
5. Keyword matcher. "ElseIf"s in a block cannot be highlighted if "Else" also exists.
  • Loading branch information
@blu3mania
blu3mania committed Apr 18, 2021
1 parent 0c37900 commit 3f247ef
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 102 deletions.
207 changes: 113 additions & 94 deletions src/Plugin/KeywordMatcher/KeywordMatcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,95 +91,104 @@ namespace papyrus {
npp_position_t currentWordStart = ::SendMessage(handle, SCI_WORDSTARTPOSITION, currentPos, true);
npp_position_t currentWordEnd = ::SendMessage(handle, SCI_WORDENDPOSITION, currentPos, true);
if (currentWordEnd > currentWordStart) {
char* word = new char[currentWordEnd - currentWordStart + 1];
auto autoCleanup = gsl::finally([&] { delete[] word; });

Sci_TextRange textRange {
.chrg = {
.cpMin = static_cast<Sci_PositionCR>(currentWordStart),
.cpMax = static_cast<Sci_PositionCR>(currentWordEnd)
},
.lpstrText = word
};
::SendMessage(handle, SCI_GETTEXTRANGE, 0, reinterpret_cast<LPARAM>(&textRange));

std::string currentWord(word);
if (utility::compare(currentWord, "Function")) {
if (settings.enabledKeywords & KEYWORD_FUNCTION) {
match(textRange.chrg, { "EndFunction", "Native" });
}
} else if (utility::compare(currentWord, "EndFunction") || utility::compare(currentWord, "Native")) {
if (settings.enabledKeywords & KEYWORD_FUNCTION) {
match(textRange.chrg, { "Function" }, false);
}
} else if (utility::compare(currentWord, "Struct")) {
if (settings.enabledKeywords & KEYWORD_STRUCT) {
match(textRange.chrg, { "EndStruct" });
}
} else if (utility::compare(currentWord, "EndStruct")) {
if (settings.enabledKeywords & KEYWORD_STRUCT) {
match(textRange.chrg, { "Struct" }, false);
}
} else if (utility::compare(currentWord, "Property")) {
if (settings.enabledKeywords & KEYWORD_PROPERTY) {
match(textRange.chrg, { "EndProperty", "Auto", "AutoReadOnly" });
}
} else if (utility::compare(currentWord, "EndProperty") || utility::compare(currentWord, "Auto") || utility::compare(currentWord, "AutoReadOnly")) {
if (settings.enabledKeywords & KEYWORD_PROPERTY) {
match(textRange.chrg, { "Property" }, false);
}
} else if (utility::compare(currentWord, "Group")) {
if (settings.enabledKeywords & KEYWORD_GROUP) {
match(textRange.chrg, { "EndGroup" });
}
} else if (utility::compare(currentWord, "EndGroup")) {
if (settings.enabledKeywords & KEYWORD_GROUP) {
match(textRange.chrg, { "Group" }, false);
}
} else if (utility::compare(currentWord, "State")) {
if (settings.enabledKeywords & KEYWORD_STATE) {
match(textRange.chrg, { "EndState" });
}
} else if (utility::compare(currentWord, "EndState")) {
if (settings.enabledKeywords & KEYWORD_STATE) {
match(textRange.chrg, { "State" }, false);
}
} else if (utility::compare(currentWord, "Event")) {
if (settings.enabledKeywords & KEYWORD_EVENT) {
match(textRange.chrg, { "EndEvent" });
}
} else if (utility::compare(currentWord, "EndEvent")) {
if (settings.enabledKeywords & KEYWORD_EVENT) {
match(textRange.chrg, { "Event" }, false);
}
} else if (utility::compare(currentWord, "While")) {
if (settings.enabledKeywords & KEYWORD_WHILE) {
match(textRange.chrg, "While", "EndWhile", {});
}
} else if (utility::compare(currentWord, "EndWhile")) {
if (settings.enabledKeywords & KEYWORD_WHILE) {
match(textRange.chrg, "EndWhile", "While", {}, false);
}
} else if (utility::compare(currentWord, "If")) {
if (settings.enabledKeywords & KEYWORD_IF) {
match(textRange.chrg, "If", "EndIf", (settings.enabledKeywords & KEYWORD_ELSE) ? otherFlowControlHighlightingWords : emptyWords);
}
} else if (utility::compare(currentWord, "EndIf")) {
if (settings.enabledKeywords & KEYWORD_IF) {
match(textRange.chrg, "EndIf", "If", (settings.enabledKeywords & KEYWORD_ELSE) ? otherFlowControlHighlightingWords : emptyWords, false);
}
} else if (utility::compare(currentWord, "Else") || utility::compare(currentWord, "ElseIf")) {
if ((settings.enabledKeywords & KEYWORD_IF) && (settings.enabledKeywords & KEYWORD_ELSE)) {
match(textRange.chrg, "If", "EndIf", otherFlowControlHighlightingWords);
match(textRange.chrg, "EndIf", "If", otherFlowControlHighlightingWords, false);
int style = static_cast<int>(::SendMessage(handle, SCI_GETSTYLEAT, currentWordStart, 0));
bool isKeyword = Lexer::isKeyword(style);
bool isFlowControl = Lexer::isFlowControl(style);
if (isKeyword || isFlowControl) {
char* word = new char[currentWordEnd - currentWordStart + 1];
auto autoCleanup = gsl::finally([&] { delete[] word; });

Sci_TextRange textRange {
.chrg = {
.cpMin = static_cast<Sci_PositionCR>(currentWordStart),
.cpMax = static_cast<Sci_PositionCR>(currentWordEnd)
},
.lpstrText = word
};
::SendMessage(handle, SCI_GETTEXTRANGE, 0, reinterpret_cast<LPARAM>(&textRange));

std::string currentWord(word);
if (isKeyword) {
if (utility::compare(currentWord, "Function")) {
if (settings.enabledKeywords & KEYWORD_FUNCTION) {
matchKeyword(textRange.chrg, { "EndFunction", "Native" });
}
} else if (utility::compare(currentWord, "EndFunction") || utility::compare(currentWord, "Native")) {
if (settings.enabledKeywords & KEYWORD_FUNCTION) {
matchKeyword(textRange.chrg, { "Function" }, false);
}
} else if (utility::compare(currentWord, "Struct")) {
if (settings.enabledKeywords & KEYWORD_STRUCT) {
matchKeyword(textRange.chrg, { "EndStruct" });
}
} else if (utility::compare(currentWord, "EndStruct")) {
if (settings.enabledKeywords & KEYWORD_STRUCT) {
matchKeyword(textRange.chrg, { "Struct" }, false);
}
} else if (utility::compare(currentWord, "Property")) {
if (settings.enabledKeywords & KEYWORD_PROPERTY) {
matchKeyword(textRange.chrg, { "EndProperty", "Auto", "AutoReadOnly" });
}
} else if (utility::compare(currentWord, "EndProperty") || utility::compare(currentWord, "Auto") || utility::compare(currentWord, "AutoReadOnly")) {
if (settings.enabledKeywords & KEYWORD_PROPERTY) {
matchKeyword(textRange.chrg, { "Property" }, false);
}
} else if (utility::compare(currentWord, "Group")) {
if (settings.enabledKeywords & KEYWORD_GROUP) {
matchKeyword(textRange.chrg, { "EndGroup" });
}
} else if (utility::compare(currentWord, "EndGroup")) {
if (settings.enabledKeywords & KEYWORD_GROUP) {
matchKeyword(textRange.chrg, { "Group" }, false);
}
} else if (utility::compare(currentWord, "State")) {
if (settings.enabledKeywords & KEYWORD_STATE) {
matchKeyword(textRange.chrg, { "EndState" });
}
} else if (utility::compare(currentWord, "EndState")) {
if (settings.enabledKeywords & KEYWORD_STATE) {
matchKeyword(textRange.chrg, { "State" }, false);
}
} else if (utility::compare(currentWord, "Event")) {
if (settings.enabledKeywords & KEYWORD_EVENT) {
matchKeyword(textRange.chrg, { "EndEvent" });
}
} else if (utility::compare(currentWord, "EndEvent")) {
if (settings.enabledKeywords & KEYWORD_EVENT) {
matchKeyword(textRange.chrg, { "Event" }, false);
}
}
} else { // isFlowControl
if (utility::compare(currentWord, "While")) {
if (settings.enabledKeywords & KEYWORD_WHILE) {
matchFlowControl(textRange.chrg, "While", "EndWhile", {});
}
} else if (utility::compare(currentWord, "EndWhile")) {
if (settings.enabledKeywords & KEYWORD_WHILE) {
matchFlowControl(textRange.chrg, "EndWhile", "While", {}, false);
}
} else if (utility::compare(currentWord, "If")) {
if (settings.enabledKeywords & KEYWORD_IF) {
matchFlowControl(textRange.chrg, "If", "EndIf", (settings.enabledKeywords & KEYWORD_ELSE) ? otherFlowControlHighlightingWords : emptyWords);
}
} else if (utility::compare(currentWord, "EndIf")) {
if (settings.enabledKeywords & KEYWORD_IF) {
matchFlowControl(textRange.chrg, "EndIf", "If", (settings.enabledKeywords & KEYWORD_ELSE) ? otherFlowControlHighlightingWords : emptyWords, false);
}
} else if (utility::compare(currentWord, "Else") || utility::compare(currentWord, "ElseIf")) {
if ((settings.enabledKeywords & KEYWORD_IF) && (settings.enabledKeywords & KEYWORD_ELSE)) {
matchFlowControl(textRange.chrg, "If", "EndIf", otherFlowControlHighlightingWords);
matchFlowControl(textRange.chrg, "EndIf", "If", otherFlowControlHighlightingWords, false);
}
}
}
}
}
}
}
}

void KeywordMatcher::match(Sci_CharacterRange currentWordPos, word_list_t matchingWords, bool searchForward) {
void KeywordMatcher::matchKeyword(Sci_CharacterRange currentWordPos, word_list_t matchingWords, bool searchForward) {
SavedSearch savedSearch(handle);
Sci_PositionCR searchStart = searchForward ? currentWordPos.cpMax : currentWordPos.cpMin;
Sci_PositionCR searchEnd = searchForward ? docLength : 0;
Expand All @@ -188,7 +197,7 @@ namespace papyrus {

// Find the one closest to current word in matching words list
for (const auto& matchingWord : matchingWords) {
auto found = findText(matchingWord, searchStart, searchEnd, searchForward);
auto found = findText(matchingWord, searchStart, searchEnd, SearchWordType::Keyword, searchForward);
if (found.cpMin != -1) {
if (searchForward) {
matchedStart = min(matchedStart, found.cpMin);
Expand All @@ -209,7 +218,7 @@ namespace papyrus {
}
}

void KeywordMatcher::match(Sci_CharacterRange currentWordPos, const char* currentWord, const char* matchingWord, word_list_t otherWords, bool searchForward) {
void KeywordMatcher::matchFlowControl(Sci_CharacterRange currentWordPos, const char* currentWord, const char* matchingWord, word_list_t otherWords, bool searchForward) {
SavedSearch savedSearch(handle);
result_list_t otherWordsPosList;
auto found = matchFlowControl(currentWordPos, currentWord, matchingWord, otherWords, otherWordsPosList, searchForward);
Expand All @@ -221,7 +230,7 @@ namespace papyrus {
::SendMessage(handle, SCI_INDICATORFILLRANGE, pos.cpMin, pos.cpMax - pos.cpMin);
}

if (found.cpMin != -1) {
if (matched) {
::SendMessage(handle, SCI_INDICATORFILLRANGE, found.cpMin, found.cpMax - found.cpMin);
}
}
Expand All @@ -232,27 +241,32 @@ namespace papyrus {

while ((searchForward && searchStart < searchEnd) || (!searchForward && searchStart > searchEnd)) {
// Find the matching flow control closest to current word
auto foundMatchingWord = findText(matchingWord, searchStart, searchEnd, searchForward);
auto foundMatchingWord = findText(matchingWord, searchStart, searchEnd, SearchWordType::FlowControl, searchForward);
if (foundMatchingWord.cpMin == -1) {
// Can't find matching flow control
return foundMatchingWord;
}

// Find the closest flow control of current word
auto foundComparison = findText(currentWord, searchStart, searchEnd, searchForward);
auto foundComparison = findText(currentWord, searchStart, searchEnd, SearchWordType::FlowControl, searchForward);

// Check if the found matching word is closer
if (foundComparison.cpMin == -1 || (searchForward && foundMatchingWord.cpMin < foundComparison.cpMin) || (!searchForward && foundMatchingWord.cpMin > foundComparison.cpMin)) {
// In this case, the found matching flow control is the true match. Add other highlighting words from current pos to matching word pos
findWords(searchStart, searchForward ? foundMatchingWord.cpMin : foundMatchingWord.cpMax, otherWords, otherWordsPosList, searchForward);
findWords(searchStart, searchForward ? foundMatchingWord.cpMin : foundMatchingWord.cpMax, otherWords, otherWordsPosList, SearchWordType::FlowControl, searchForward);
return foundMatchingWord;
}

// Nested flow control. Use recursive method to find the end position of nested block.
// Add any other highlighting words from current pos to start of nested block, and skip highlighting other words inside nested block
findWords(searchStart, searchForward ? foundComparison.cpMin : foundComparison.cpMax, otherWords, otherWordsPosList, searchForward);
findWords(searchStart, searchForward ? foundComparison.cpMin : foundComparison.cpMax, otherWords, otherWordsPosList, SearchWordType::FlowControl, searchForward);
auto nestedFlowControlEnd = matchFlowControl(foundComparison, currentWord, matchingWord, {}, otherWordsPosList, searchForward);

// If nested control is open-ended, return as well
if (nestedFlowControlEnd.cpMin == -1) {
return nestedFlowControlEnd;
}

// Use nested flow control block's end postion as new start search position
searchStart = searchForward ? nestedFlowControlEnd.cpMax : nestedFlowControlEnd.cpMin;
}
Expand All @@ -261,7 +275,7 @@ namespace papyrus {
return Sci_CharacterRange { .cpMin = -1 };
}

Sci_CharacterRange KeywordMatcher::findText(const char* text, Sci_PositionCR start, Sci_PositionCR end, bool searchForward, bool allowComments, int searchFlags) {
Sci_CharacterRange KeywordMatcher::findText(const char* text, Sci_PositionCR start, Sci_PositionCR end, SearchWordType searchWordType, bool searchForward, int searchFlags) {
Sci_TextToFind search {
.chrg = {
.cpMin = start,
Expand All @@ -270,7 +284,10 @@ namespace papyrus {
.lpstrText = text
};
while (::SendMessage(handle, SCI_FINDTEXT, searchFlags, reinterpret_cast<LPARAM>(&search)) != -1) {
if (!allowComments && Lexer::isComment(static_cast<int>(::SendMessage(handle, SCI_GETSTYLEAT, search.chrgText.cpMin, 0)))) {
// Search result has to be either keyword or flow control, depending on search word type
int style = static_cast<int>(::SendMessage(handle, SCI_GETSTYLEAT, search.chrgText.cpMin, 0));
if ((searchWordType == SearchWordType::Keyword && !Lexer::isKeyword(style)) || (searchWordType == SearchWordType::FlowControl && !Lexer::isFlowControl(style))) {
// Not in correct style, likely a comment or string, search again
search.chrg.cpMin = searchForward ? search.chrgText.cpMax : search.chrgText.cpMin;
} else {
// Found
Expand All @@ -283,18 +300,20 @@ namespace papyrus {
return search.chrgText;
}

void KeywordMatcher::findWords(Sci_PositionCR start, Sci_PositionCR end, word_list_t words, result_list_t& foundPosList, bool searchForward) {
void KeywordMatcher::findWords(Sci_PositionCR start, Sci_PositionCR end, word_list_t words, result_list_t& foundPosList, SearchWordType searchWordType, bool searchForward) {
for (const auto& word : words) {
Sci_PositionCR searchStart = start;
Sci_PositionCR searchEnd = end;
while (true) {
auto found = findText(word, start, end, searchForward);
auto found = findText(word, searchStart, searchEnd, searchWordType, searchForward);
if (found.cpMin == -1) {
// No more matches
break;
}
foundPosList.push_back(found);

// Search again
start = searchForward ? found.cpMax : found.cpMin;
searchStart = searchForward ? found.cpMax : found.cpMin;
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions src/Plugin/KeywordMatcher/KeywordMatcher.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,17 @@ namespace papyrus {
void clear();

private:
enum class SearchWordType {
Keyword,
FlowControl
};

void match();
void match(Sci_CharacterRange currentWordPos, word_list_t matchingWords, bool searchForward = true);
void match(Sci_CharacterRange currentWordPos, const char* currentWord, const char* matchingWord, word_list_t otherWords, bool searchForward = true);
void matchKeyword(Sci_CharacterRange currentWordPos, word_list_t matchingWords, bool searchForward = true);
void matchFlowControl(Sci_CharacterRange currentWordPos, const char* currentWord, const char* matchingWord, word_list_t otherWords, bool searchForward = true);
Sci_CharacterRange matchFlowControl(Sci_CharacterRange currentWordPos, const char* currentWord, const char* matchingWord, word_list_t otherWords, result_list_t& otherWordsPosList, bool searchForward = true);
Sci_CharacterRange findText(const char* text, Sci_PositionCR start, Sci_PositionCR end, bool searchForward = true, bool allowComments = false, int searchFlags = SCFIND_WHOLEWORD);
void findWords(Sci_PositionCR start, Sci_PositionCR end, word_list_t words, result_list_t& foundPosList, bool searchForward = true);
Sci_CharacterRange findText(const char* text, Sci_PositionCR start, Sci_PositionCR end, SearchWordType searchWordType, bool searchForward = true, int searchFlags = SCFIND_WHOLEWORD);
void findWords(Sci_PositionCR start, Sci_PositionCR end, word_list_t words, result_list_t& foundPosList, SearchWordType searchWordType, bool searchForward = true);

void setupIndicator();
void showIndicator();
Expand Down
Loading

0 comments on commit 3f247ef

Please sign in to comment.