Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intellisense not working as expected #74

Closed
megakraken opened this issue Feb 8, 2024 · 6 comments
Closed

Intellisense not working as expected #74

megakraken opened this issue Feb 8, 2024 · 6 comments

Comments

@megakraken
Copy link

Hello,

when typing something like File. I would expect the intellisense popup to appear with all members for File and then as I proceed to type see the list gradually be narrowed down to the matching members. Well, just like in Visual Studio.

However the popup won't appear automatically after typing . and the only way I can get it to show is by pressing CTRL + Space. Also the list won't narrow down as I continue typing and I have to manually scroll through it to select the member I want.

Here's a video that illustrates what I mean:

intellisense.mp4

Is this how this plugin is supposed to work or is there something wrong? I didn't change any of the settings and it's a fresh install of notepad++ without any other plugins running.

For what it's worth I'm using plugin version 1.7.24.0 (x64) and notepad++ version v8 (x64) as suggested in #71 because I'm wanting to target the .NET Framework.

Thanks for any help!

@oleg-shilo
Copy link
Owner

oleg-shilo commented Feb 9, 2024

Thankyou @megakraken,

No, it's not how it's supposed to work. Unfortunately, some Notepad++ internal changes interfere with the plugin's business logic. It might be happening again as I have updated N++ and started having the same problem.

Thank you for reporting. I will have a look at it.

Upd: it looks like now N++ is stealing the focus from the plugin popup so it does not receive the keystrokes so filtering is not triggered.

Will see how it can be worked around.

@oleg-shilo oleg-shilo added the bug label Feb 9, 2024
oleg-shilo added a commit that referenced this issue Feb 9, 2024
- The default of `AutoSelectFirstSuggestion` to true. To make it consistent with VS. It can always be changed in the config file.
@oleg-shilo
Copy link
Owner

The things are happen to be more complicated.
While the loss of input focus indeed prevents keystrokes from going to the plugin it is actually Scintilla who is a suspect here (or NPP/Scintilla interop).

For whatever reason the SciMsg.SCN_CHARADDED notifications are coming with the char pointer set to zero.

image

Fortunately, it's possible to find out what character was typed by other means but.... it is not very elegant.

Though the user experience will remain the same.
I will publish the update in a day or two.

@oleg-shilo
Copy link
Owner

Reopening after an accidental closing.

Done fix is available in the latest release v2.0.4.0 (Releases page)
The release has also been published on nppPluginList but the next release of the plugins updates up to the N++ team.

If you do not want to wait for the next N++ release, you can update the plugin from the Plugin-settings page.
The release source is:

image

@rdipardo
Copy link

@oleg-shilo

While the loss of input focus indeed prevents keystrokes from going to the plugin it is actually Scintilla who is a suspect here (or NPP/Scintilla interop).

For whatever reason the SciMsg.SCN_CHARADDED notifications are coming with the char pointer set to zero.

Just a quick postscript here. The underlying issue really is NPP/Scintilla interop. Specifically, the .NET wrapper for notifications has wrongly sized fields for 64-bit binary compatibility.

In a 64-bit process, the editor fills nc.position with 64 bits of data (because the C++ member type, Sci_Position, is pointer-width). But the .NET member type is int, which only holds 32 bits. The spillover data clobbers the field beside it (nc.character), and so on, clobbering all the neighbouring memory blocks in turn. You should not trust any of those fields to have sensible values in 64-bit mode.

I would not worry about it now since the reported bug is fixed well enough.

If memory safety issues come up in the future, have a look at this related issue, and the quick patch that resolved it.

@oleg-shilo
Copy link
Owner

@rdipardo, thank you so much for a detailed explanation. It makes perfect sense.

I will update the struct ScNotification as you suggested.

It's great to have you in this conversation. It's not the first time you have assisted me with the intricacies of the Scintilla interface.
Greatly appreciated.

@rdipardo
Copy link

rdipardo commented Feb 14, 2024

It makes perfect sense.

If only the people who made Plugin.NET so popular had thought so, too 😔. Somebody had the bright idea of making plugins "easy" to develop. The ironic truth is there's nothing easy about safely marshalling .NET objects into C-like structures. By now there must be thousands and thousands of .NET plugins on end-user PCs, all of them silently writing data out of bounds every time Scintilla emits a notification...

oleg-shilo added a commit that referenced this issue May 25, 2024
  Triggered by #74 "Intellisense not working as expected"
oleg-shilo added a commit that referenced this issue May 26, 2024
----
- Embedded CS-Script engine is updated to v4.8.16.0
- Configuration Dialog: Added support for installing external CS-Script engine from WinGet and NuGet.
- Updated plugin hosting solution to address the Scintilla .NET Interface issue (#74).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants