Bug Fixed Autocomplete ignore case by anil-shrestha · Pull Request #2382 · notepad-plus-plus/notepad-plus-plus

anil-shrestha · 2016-10-03T06:47:31Z

In notepad++ APIs folder there is vb.xml, it contains keywords which has space character in name value
for example <KeyWord name="If Then Else" />.

To reproduce the bug:
open new document
Goto Language and choose Visual Basic
start typing If autocomplete box will shows If Then Else in separate line
while the expected behavior is to show in single line

Similarly, Autocompete box only shows the list of the values which matches the case of the input value
while the expected behavior is to ignore case.

Now even if we put <Environment ignoreCase="yes" > (taken reference from c.xml) in vb.xml, this behavior is not changed.

Both the bugs are fixed in this PR.

anil-shrestha · 2016-10-03T07:12:05Z

I have build with vs2015

MAPJe71

Please read and apply Guidelines for pull requests.

anil-shrestha

code indentation and formatting changed

dail8859 · 2016-10-03T14:08:41Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

 {
 	const size_t bufSize = 256;
-
+	if (isAllDigits(beginChars))


Forcing this upon a user isn't a good idea. They may well want numbers to be autocompleted. I have PR #2378 that does this exact same thing but allows the user to enable it or not.

dail8859 · 2016-10-03T14:13:06Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

-
+		if (_ignoreCase)
+		{
+			std::sort(_keyWordArray.begin(), _keyWordArray.end(), [](const auto& lhs, const auto& rhs){


This is an exact copy of the std::sort() call on line 120 and 376. You can probably put this into a separate function and would make the code a bit more readable, e.g. having a call to sortCaseInsensitive(...) or whatever.

dail8859 · 2016-10-04T18:52:43Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

+
+		//if (_keyWordArray[i].compare(0, len, beginChars) == 0)
+		//if (keytemp.compare(0, len, beginchartmp) == 0)
+		if (((keytemp.compare(0, len, beginchartmp) == 0) && _ignoreCase) || (_keyWordArray[i].compare(0, len, beginChars) == 0))


This is inefficient because it always transforms the two strings even if _ignoreCase is false.

changes done

dail8859 · 2016-10-04T18:57:47Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

+void AutoCompletion::sortKeyWords()
+{
+	if (_ignoreCase) {
+		std::sort(_keyWordArray.begin(), _keyWordArray.end(), [](const auto& lhs, const auto& rhs) {


This does not work with VS 2013 (the official build system).

So ... that's probably why the AppVeyor build fails (hint for OP).

I don't have VS 2013. Please help.
(May be its time to upgrade the official build system, if not remove the VS 2015 proj file from source)

FYI:

it's possible to create code with VS2015 that also compiles with VS2013.

there's a free edition of VS2013.

MAPJe71 · 2016-10-06T12:23:02Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

 		}
-		else if (canStop)
-		{
+		else if (canStop) {


This change reverses code that complies to the Style Guide.

MAPJe71 · 2016-10-06T12:23:13Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp


-	if (_ignoreCase)
-	{
+	if(_ignoreCase){


This change reverses code that complies to the Style Guide.

MAPJe71 · 2016-10-06T12:24:53Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

+			return result.second != rhs.cend() && (result.first == lhs.cend() || tolower(*result.first) < tolower(*result.second));
+		});
+	}
+	else {


And again ... code style!

MAPJe71

Don't change the default build settings of the project!

MAPJe71

Revert changes to AppVeyor.yml and make sure it builds with VS2013!

anil-shrestha · 2016-10-09T09:48:33Z

I have made the changes to be build with VS2013. But dont know how to resolve the conflicts. Plz help.

anil-shrestha

changed to be able to build on VS2013

cmeriaux · 2016-10-09T11:01:05Z

please squash all yours commits.

git feth --all
git rebase -i upstream/master

then change "pick" command to "squash" command on each commits

Once it's ok, push with force option. It will update the PR.

git push origin master -f

anil-shrestha · 2016-10-17T04:39:33Z

Any updates ?

MAPJe71 · 2016-10-17T12:13:51Z

One commit per PR!
See post by @cmeriaux to solve.

dail8859 · 2016-10-17T12:41:59Z

PowerEditor/src/ScitillaComponent/AutoCompletion.cpp

-		if (_keyWordArray[i].compare(0, len, beginChars) == 0)
+
+		bool _ignoreCaseMatch = false;
+		if (_ignoreCase)


This if block creates lots of copies of strings. Is there a better way to compare them? Can you use _tcsicmp()? If so could you also use it in AutoCompletion::sortKeyWords()?

dail8859 · 2016-10-17T12:52:10Z

PowerEditor/src/ScitillaComponent/AutoCompletion.h

 	void callTipClick(int direction);
 	void getCloseTag(char *closeTag, size_t closeTagLen, size_t caretPos, bool isHTML);
-
+	void sortKeyWords();


No reason to have this public.

anil-shrestha

squashed all commits to one

anil-shrestha · 2016-10-23T03:33:34Z

Any Updates ?

cmeriaux · 2016-10-23T19:57:34Z

Your release note is wrong. You should remove

updated to VS 2015
Update appveyor.yml
Resolved Conflicts

anil-shrestha

removed unnecessary commit messages

anil-shrestha · 2016-10-26T14:56:40Z

Any updates?

anil-shrestha · 2016-10-31T06:58:09Z

any updates?

anil-shrestha · 2016-11-03T16:14:00Z

Please update if anything needs to be done so that you can accept this change.

cmeriaux · 2016-11-03T17:46:23Z

For me, nothing to update.
You must be patient before being integrated

donho · 2016-11-13T18:58:44Z

@anil-shrestha Please describe the bugs you fixed in detail or refer to an issue (how to reproduce it) . Your description is confused.

anil-shrestha · 2016-11-17T10:10:30Z

I have changed the description. Please advise if it is ok.

anil-shrestha · 2016-12-07T09:23:30Z

I am not able to squash. Please help

AraHaan · 2016-12-07T09:36:41Z

Well for me squashing only works with 1 commit only at a time. I had that issue to so that means that any multiple commits from after that would be discarded so back up any of your other changes and then commit them into one locally and then squash that before syncing.

(I know its dumb design really)

But rebase all your changes from after 4c23de6 like so:

git fetch --all
git rebase -i 4c23de61b48b46a89fc98e78721e1707fe22514e

Any of your comits from before that from this PR would have to be backed up then deleted on the files that they changed just so you know before squashing as well.

anil-shrestha · 2016-12-07T09:55:52Z

Please send me commands.

dail8859 · 2016-12-08T01:30:49Z

@anil-shrestha It seems to be working fine for me now, thanks.

anil-shrestha · 2016-12-08T04:01:42Z

@dail8859 I am not able to squash. Please help

dail8859 · 2016-12-08T04:06:44Z

It's a bit complicated since you made commits directly on your master branch, but if you are ok with it I can fix it for you. Keep in mind I'll have to force push it and you will probably have to reset your local master branch.

anil-shrestha · 2016-12-08T04:09:16Z

Please go ahead

dail8859 · 2016-12-08T04:12:36Z

Should be fixed now. I rebased the commits and squashed them. I force pushed directly to your master branch in your repository.

anil-shrestha · 2016-12-08T04:16:42Z

Thank you. Can you send me the commands you used.

dail8859 · 2016-12-08T04:17:42Z

I did it all with the TortoiseGit GUI, so not 100% sure the exact commands needed.

cmeriaux · 2016-12-08T06:28:52Z

commands is git rebase -i HEAD~6 ( 6 means the number of commit to rebase)
Then use squash option instead of pick

See the helps here :
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#Squashing-Commits

anil-shrestha · 2016-12-15T08:30:30Z

Any Updates?

simon33-2 · 2016-12-19T08:15:35Z

I also would like this bug fixed and is it possible to further enhance it to allow:

multi line auto complete blocks to be inserted
a cursor position after autocomplete is applied, rather than always placing the cursor at the end of the inserted block at present.

I trust you can see how this would be useful, but just in case, let's say you are writing C and have a standard of always including braces ( {} pairs ).

If I type "do" I want:

do {
<indent><cursor>
} while();

or "if":

if( <cursor> ) {
<indent><end of line>
}

Doesn't work quite as well for "for" statements but works well in certain languages for certain expressions. Typing block closing delimiters shouldn't really be necessary given that they must always be present in certain situations.

dail8859 · 2016-12-19T14:11:25Z

@simon33-2 That goes a bit more beyond what would be considered "auto-completion". Most editors consider this something like "snippets". There are a couple of Notepad++ plugins that does exactly what you describe (e.g. SnippetsPlus, FingerText, and others).

anil-shrestha · 2017-01-03T14:36:48Z

Any Updates?
How many more releases before this PR gets accepted?

anil-shrestha · 2017-01-10T07:04:53Z

@donho Can you please let me know what exactly has happened ?

anil-shrestha · 2017-01-18T07:05:50Z

Anyone please help

donho · 2017-02-05T14:12:14Z

Autocompletion feature is a generic function for all surppoted languages, so the modification for adapting only for VB will make autocompletion break for the other languages.

vb.xml needs to be modified for removing all the entries contain space, and make all keywords in UPPERCAS or lowercase according the convention. I'll see what can I do for make ignore the case of keyin to trigger auto-completion.

anil-shrestha · 2017-02-06T04:46:33Z

@donho vb.xml was just an example. I have already tested for c.xml and other xmls and my UDL xml as well. And the problem is not only upper or lower case, it is the Keyword containing space character too.

Please check in npp++ 5.8.3 which I am currently using which does not have above bugs. Due to these bugs, I am not able to upgrade to latest versions :(

Please note that removing keywords containing space from vb.xml is not the solution as there are many other users like me who are using their own UDL and their Keywords contain space character.

anil-shrestha · 2017-02-17T03:44:16Z

@donho are you looking into it? Please consider my previous comment

GrumpyGopher · 2017-04-14T13:52:52Z

I was wondering where this left off? I had some issues with auto complete that I believe are related to this fix. I'm using VBScript with IgnoreCase set to Yes and additionalWordChar set to "."

Current sort seems to put upper case first. For example if I have a list of keywords containing Janice and JJAbrams when I type J the list will display like:

JJAbrams
Janice

It seems that the first letter is not ignoring case so if I type the lower case "j" I don't get the dialog at all.
But if I type the upper case "J" will still let me type Janice without the Auto complete dialog disappearing. But if my autocomplete keyword list contains the additionalWordChar the dialog will close and not let me type the 2nd term. For example if keywords are object.JJAbrams and object.Janice once I type the "a" in object.Janice the autoComplete dialog goes away.

GrumpyGopher · 2017-04-14T21:18:45Z

I was able to do some testing with this today and it does solve my problem. I was using what @anil-shrestha did without the space character logic. It looks like the issue was closed because of the space logic but would it be possible to submit the change with just the ignore case sorting and comparing? I have an updated version of AutoCompletion.cpp if you need me to do something let me know.

AraHaan · 2017-04-14T21:25:08Z

Or, what Notepad++ can do is provide an configuration setting to enable this if they desire. And yes this can bite me as well. I just never ran into it yet (or know if I did) when I would use Notepad++ for my python coding.

Although the 1 thing python I would like is the ability to have Notepad++ know if python code breaks PEP8 (As an optional setting) that would also check for new lines at end of file.

anil-shrestha · 2017-05-11T14:24:41Z

@tr12345 I don't know what went wrong and why the PR got closed. I have emailed @donho too. However there is no reply. Hope some day @donho reconsiders ...

update to Scinitlla Release 5.3.6 (https://www.scintilla.org/scintilla536.zip) Released 26 July 2023. Redraw calltip after showing as didn't update when size of new text exactly same as previous. Feature notepad-plus-plus#1486. On Win32 fix reverse arrow cursor when scaled. Bug notepad-plus-plus#2382. On Win32 hide cursor when typing if that system preference has been chosen. Bug notepad-plus-plus#2333. On Win32 and Qt, stop aligning IME candidate window to target. It is now always aligned to start of composition string. This undoes part of feature notepad-plus-plus#1300. Feature notepad-plus-plus#1488, Bug notepad-plus-plus#2391, Feature notepad-plus-plus#1300. On Qt, for IMEs, update micro focus when selection changes. This may move the location of IME popups to align with the caret. On Qt, implement replacement for IMEs which may help with actions like reconversion. This is similar to delete-surrounding on GTK. and Lexilla Release 5.2.6 (https://www.scintilla.org/lexilla526.zip) Released 26 July 2023. Include empty word list names in value returned by DescribeWordListSets and SCI_DESCRIBEKEYWORDSETS. Issue notepad-plus-plus#175, Pull request notepad-plus-plus#176. Bash: style here-doc end delimiters as SCE_SH_HERE_DELIM instead of SCE_SH_HERE_Q. Issue notepad-plus-plus#177. Bash: allow '$' as last character in string. Issue notepad-plus-plus#180, Pull request notepad-plus-plus#181. Bash: fix state after expansion. Highlight all numeric and file test operators. Don't highlight dash in long option as operator. Issue notepad-plus-plus#182, Pull request notepad-plus-plus#183. Bash: strict checking of special parameters ($*, $@, $$, ...) with property lexer.bash.special.parameter to specify valid parameters. Issue notepad-plus-plus#184, Pull request notepad-plus-plus#186. Bash: recognize keyword before redirection operators (< and >). Issue notepad-plus-plus#188, Pull request notepad-plus-plus#189. Errorlist: recognize Bash diagnostic messages. HTML: allow ASP block to terminate inside line comment. Issue notepad-plus-plus#185. HTML: fix folding with JSP/ASP.NET <%-- comment. Issue notepad-plus-plus#191. HTML: fix incremental styling of multi-line ASP.NET directive. Issue notepad-plus-plus#191. Matlab: improve arguments blocks. Add support for multiple arguments blocks. Prevent "arguments" from being keyword in function declaration line. Fix semicolon handling. Pull request notepad-plus-plus#179. Visual Prolog: add support for embedded syntax with SCE_VISUALPROLOG_EMBEDDED and SCE_VISUALPROLOG_PLACEHOLDER. Styling of string literals changed with no differentiation between literals with quotes and those that are prefixed with "@". Quote characters are in a separate style (SCE_VISUALPROLOG_STRING_QUOTE) to contents (SCE_VISUALPROLOG_STRING). SCE_VISUALPROLOG_CHARACTER, SCE_VISUALPROLOG_CHARACTER_TOO_MANY, SCE_VISUALPROLOG_CHARACTER_ESCAPE_ERROR, SCE_VISUALPROLOG_STRING_EOL_OPEN, and SCE_VISUALPROLOG_STRING_VERBATIM_SPECIAL were removed (replaced with SCE_VISUALPROLOG_UNUSED[1-5]). Pull request notepad-plus-plus#178.

update to Scinitlla Release 5.3.6 (https://www.scintilla.org/scintilla536.zip) Released 26 July 2023. Redraw calltip after showing as didn't update when size of new text exactly same as previous. Feature notepad-plus-plus#1486. On Win32 fix reverse arrow cursor when scaled. Bug notepad-plus-plus#2382. On Win32 hide cursor when typing if that system preference has been chosen. Bug notepad-plus-plus#2333. On Win32 and Qt, stop aligning IME candidate window to target. It is now always aligned to start of composition string. This undoes part of feature notepad-plus-plus#1300. Feature notepad-plus-plus#1488, Bug notepad-plus-plus#2391, Feature notepad-plus-plus#1300. On Qt, for IMEs, update micro focus when selection changes. This may move the location of IME popups to align with the caret. On Qt, implement replacement for IMEs which may help with actions like reconversion. This is similar to delete-surrounding on GTK. and Lexilla Release 5.2.6 (https://www.scintilla.org/lexilla526.zip) Released 26 July 2023. Include empty word list names in value returned by DescribeWordListSets and SCI_DESCRIBEKEYWORDSETS. Issue notepad-plus-plus#175, Pull request notepad-plus-plus#176. Bash: style here-doc end delimiters as SCE_SH_HERE_DELIM instead of SCE_SH_HERE_Q. Issue notepad-plus-plus#177. Bash: allow '$' as last character in string. Issue notepad-plus-plus#180, Pull request notepad-plus-plus#181. Bash: fix state after expansion. Highlight all numeric and file test operators. Don't highlight dash in long option as operator. Issue notepad-plus-plus#182, Pull request notepad-plus-plus#183. Bash: strict checking of special parameters ($*, $@, $$, ...) with property lexer.bash.special.parameter to specify valid parameters. Issue notepad-plus-plus#184, Pull request notepad-plus-plus#186. Bash: recognize keyword before redirection operators (< and >). Issue notepad-plus-plus#188, Pull request notepad-plus-plus#189. Errorlist: recognize Bash diagnostic messages. HTML: allow ASP block to terminate inside line comment. Issue notepad-plus-plus#185. HTML: fix folding with JSP/ASP.NET <%-- comment. Issue notepad-plus-plus#191. HTML: fix incremental styling of multi-line ASP.NET directive. Issue notepad-plus-plus#191. Matlab: improve arguments blocks. Add support for multiple arguments blocks. Prevent "arguments" from being keyword in function declaration line. Fix semicolon handling. Pull request notepad-plus-plus#179. Visual Prolog: add support for embedded syntax with SCE_VISUALPROLOG_EMBEDDED and SCE_VISUALPROLOG_PLACEHOLDER. Styling of string literals changed with no differentiation between literals with quotes and those that are prefixed with "@". Quote characters are in a separate style (SCE_VISUALPROLOG_STRING_QUOTE) to contents (SCE_VISUALPROLOG_STRING). SCE_VISUALPROLOG_CHARACTER, SCE_VISUALPROLOG_CHARACTER_TOO_MANY, SCE_VISUALPROLOG_CHARACTER_ESCAPE_ERROR, SCE_VISUALPROLOG_STRING_EOL_OPEN, and SCE_VISUALPROLOG_STRING_VERBATIM_SPECIAL were removed (replaced with SCE_VISUALPROLOG_UNUSED[1-5]). Pull request notepad-plus-plus#178. Fix notepad-plus-plus#13901, fix notepad-plus-plus#13911, fix notepad-plus-plus#13943, close notepad-plus-plus#13940

MAPJe71 reviewed Oct 3, 2016

View reviewed changes

anil-shrestha commented Oct 3, 2016

View reviewed changes

dail8859 reviewed Oct 3, 2016

View reviewed changes

dail8859 reviewed Oct 4, 2016

View reviewed changes

MAPJe71 reviewed Oct 6, 2016

View reviewed changes

MAPJe71 suggested changes Oct 6, 2016

View reviewed changes

MAPJe71 suggested changes Oct 7, 2016

View reviewed changes

anil-shrestha commented Oct 9, 2016

View reviewed changes

anil-shrestha force-pushed the master branch from e321497 to 1db5038 Compare October 14, 2016 06:27

dail8859 reviewed Oct 17, 2016

View reviewed changes

anil-shrestha force-pushed the master branch from b2937ad to 2d05f25 Compare October 19, 2016 06:20

anil-shrestha commented Oct 19, 2016

View reviewed changes

anil-shrestha force-pushed the master branch from 2d05f25 to fb39760 Compare October 24, 2016 06:47

anil-shrestha commented Oct 24, 2016

View reviewed changes

donho added the suspended label Nov 13, 2016

changed sortKeyWords function to use upper case

605e468

dail8859 force-pushed the master branch from 375adbe to 605e468 Compare December 8, 2016 04:11

donho closed this Feb 5, 2017

donho removed the suspended label Feb 5, 2017

Conversation

anil-shrestha commented Oct 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anil-shrestha commented Oct 3, 2016

Uh oh!

MAPJe71 left a comment

Choose a reason for hiding this comment

Uh oh!

anil-shrestha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MAPJe71 Oct 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MAPJe71 left a comment

Choose a reason for hiding this comment

Uh oh!

MAPJe71 left a comment

Choose a reason for hiding this comment

Uh oh!

anil-shrestha commented Oct 9, 2016

Uh oh!

anil-shrestha left a comment

Choose a reason for hiding this comment

Uh oh!

cmeriaux commented Oct 9, 2016

Uh oh!

anil-shrestha commented Oct 17, 2016

Uh oh!

MAPJe71 commented Oct 17, 2016

Uh oh!

dail8859 Oct 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anil-shrestha left a comment

Choose a reason for hiding this comment

Uh oh!

anil-shrestha commented Oct 23, 2016

Uh oh!

cmeriaux commented Oct 23, 2016

Uh oh!

anil-shrestha left a comment

Choose a reason for hiding this comment

anil-shrestha commented Oct 3, 2016 •

edited

Loading

MAPJe71 Oct 4, 2016 •

edited

Loading

dail8859 Oct 17, 2016 •

edited

Loading

anil-shrestha commented Nov 17, 2016 •

edited

Loading

AraHaan commented Dec 7, 2016 •

edited

Loading

dail8859 commented Dec 8, 2016 •

edited

Loading

simon33-2 commented Dec 19, 2016 •

edited

Loading