New API to fix eventual regression regarding SCN_MODIFIED for some plugins

donho

From v8.7.6, Notepad++ forwards only SCN_MODIFIED with the following 5 notification flags SC_MOD_DELETETEXT, SC_MOD_INSERTTEXT, SC_PERFORMED_UNDO, SC_PERFORMED_REDO and SC_MOD_CHANGEINDICATOR to plugins.

So if your plugin processes the events of SCN_MODIFIED other than the above 5 ones, for example SC_MOD_DELETETEXT and SC_MOD_INSERTTEXT, then it’s broken with v8.7.6 and later versions.

To remedy this “regression” of v8.7.6, a new message NPPM_ADDSCNMODIFIEDFLAGS is introduced in v8.7.7 for plugins which process the events of SCN_MODIFIED other than 5 default ones. All you need is add the notifications you need by sending this message to Notepad++, just after recieving NPPN_READY.

New message:
BOOL NPPM_ADDSCNMODIFIEDFLAGS(0, unsigned long scnMotifiedFlags2Add)

Description:
Add needed SCN_MODIFIED flags so your plugin will recieve the notification SCN_MODIFIED of these events for your specific treatments.
By default, Notepad++ only forwards SCN_MODIFIED with the following 5 flags/events SC_MOD_DELETETEXT | SC_MOD_INSERTTEXT | SC_PERFORMED_UNDO | SC_PERFORMED_REDO | SC_MOD_CHANGEINDICATOR to plugins.
If your plugin need to process other events of SCN_MODIFIED, you should add the flags you need by sending this message to Notepad++, just after recieving NPPN_READY.

wParam: 0 (not used)
lParam[in]: scnMotifiedFlags2Add- Scintilla SCN_MODIFIED flags to add.
Return TRUE

Example:

extern "C" __declspec(dllexport) void beNotified(SCNotification* notifyCode)
{
	switch (notifyCode->nmhdr.code)
	{
		case NPPN_READY:
		{
			// Add SC_MOD_BEFOREDELETE and SC_MOD_BEFOREINSERT notifications
			::SendMessage(nppData._nppHandle, NPPM_ADDSCNMODIFIEDFLAGS, 0, SC_MOD_BEFOREDELETE | SC_MOD_BEFOREINSERT); 
		}
		break;
		...
	}
	...
}

Ekopalypse

@donho
nitpicking - since these notifications are used in lparam, they are actually intptr_t and not uintptr_t.

Coises

@donho said in New API to fix eventual regression for some plugins:

From v8.7.6, Notepad++ forwards only the following 5 default notification SC_MOD_DELETETEXT | SC_MOD_INSERTTEXT | SC_PERFORMED_UNDO | SC_PERFORMED_REDO | SC_MOD_CHANGEINDICATOR to plugins.

I know what you wrote isn’t what you meant, but if taken literally that statement will cause many plugin authors to panic and/or give up hope.

There are many notifications Scintilla sends on which plugins depend. (For example, I believe any usable implementation of elastic tabstops — not just mine — must process SCN_UPDATEUI; setting custom tabstops in Scintilla is slow enough that it’s essential to do it only for visible lines. To do that, you have to monitor display changes, such as scrolling. SCN_UPDATEUI is also the reasonable place to “batch” any kind of processing that is better done all at once that one bit at a time.)

The flags you are processing in NPPM_ADDSCINTILLANOTIFS are the bits of the Event Mask for one specific notification, SCN_MODIFIED.

I don’t mean to be crabby about it… it’s just that, as written, your statement is inaccurate, and I fear it could be misinterpreted as meaning what it says.

So if your plugin processes other notifications than the above 5 ones, for example SC_MOD_DELETETEXT and SC_MOD_INSERTTEXT, then it’s broken with v8.7.6 and later version.

You gave as examples events that you say are already included. I think you meant SC_MOD_BEFOREDELETE and SC_MOD_BEFOREINSERT.

Perhaps it would be best to say that plugins should enable all SCN_MODIFIED events they wish to monitor? The five you gave are the default for now… why assume they won’t change again?

rdipardo

@Coises said in New API to fix eventual regression for some plugins:

The five [modification flags] are the default for now… why assume they won’t change again?

It would be best to expose the default flags to the plugin interface, e.g.,

#define DEFAULT_SC_MOD_FLAGS  /* SC_MOD_DELETETEXT | ...., etc. */

Then plugins can just do a bitwise operation on DEFAULT_SC_MOD_FLAGS to discover what’s available and what’s not.

If the defaults change in the future, just edit the interface header file. Since there’s not going to be a companion*_GET_SC_MOD_FLAGS message, it’s better than nothing.

Coises

@rdipardo said in New API to fix eventual regression for some plugins:

Since there’s not going to be a companion*_GET_SC_MOD_FLAGS message, it’s better than nothing.

If for some reason a plugin wants to know what event mask is set but doesn’t want to specify its own events, SCI_GETMODEVENTMASK will tell it. (Note that the reason we can’t just use SCI_SETMODEVENTMASK instead of this new message is that Notepad++ turns the mask off — sets it to zero to suppress all SCN_MODIFIED notifications — and back on under various conditions to improve efficiency. I suppose it could save the value when it does that and then restore it, but that’s not the approach that was chosen.) I’m not sure if the default event mask is set during NPPN_READY (I haven’t parsed the code that far yet); if I’m reading the code correctly, it doesn’t change during NPPM_ADDSCINTILLANOTIFS processing, so either you would get the default then or you would get nothing meaningful. You could get the end result of all plugins’ additions during any subsequent command or notification.

The intent, though, is that you just tell Notepad++ which SCN_MODIFIED events you need enabled. My suggestion was to define “best practice” as including all the events you need, even if they are currently included in the defaults.

Mark Olson

I’m glad @donho is responding so quickly to the needs of the plugin community. Thanks!

@Coises said in New API to fix eventual regression for some plugins:

I know what you wrote isn’t what you meant, but if taken literally that statement will cause many plugin authors to panic and/or give up hope.

There are many notifications Scintilla sends on which plugins depend.

Agree with this. I think that before this API is released in the wild, the message should be renamed to NPPM_ADDSCNMODIFIEDFLAGS. That name is about the same length and more clearly communicates the intent of the message.

Michael Vincent

@Coises said in New API to fix eventual regression for some plugins:

I know what you wrote isn’t what you meant, but if taken literally that statement will cause many plugin authors to panic and/or give up hope.

I was there momentarily, but your post explains it perfectly and the only time in PythonScript scripts or my own C++ plugins I use SCN_MODIFIED is when I’m looking for SC_MOD_INSERT/DELETETEXT. Indeed a quick check of functionality in N++ 8.7.6 shows I’m still working fine.

Also, your SCI_GETMODEVENTMASK hint is useful. I used NppExec to get it:

SCI_SENDMSG  SCI_GETMODEVENTMASK
echo $(MSG_RESULT)

and in N++ pre 8.7.6 it is = 8388607(which I didn’t bother “decoding”),
while in N++ 8.7.6 it is = 16483 (which from your link equates to the 5 messages N++ still forwards by default).

Cheers.

donho

@Coises
You’re absolutely right about it - it was not accurate at all.
I’ve reworded the announcement title & content to make it more consistent and clearer.

Thank you everyone for providing your valuable opinions.
Please let me know if there are any other points that could be improved.

Coises

@donho said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

Please let me know if there are any other points that could be improved.

One observation and one technical question.

Observation:

A small point, but the comments here still use adding SC_MOD_DELETETEXT and SC_MOD_INSERTTEXT as an example, when they are already part of the default. I think you meant to use SC_MOD_BEFOREDELETE and SC_MOD_BEFOREINSERT as examples. Also — and I understand you might not agree — would it not be better, as a matter of “future-proofing,” to illustrate best practice as explicitly enabling all SCN_MODIFIED events the plugin plans to use, even if they are (currently) part of the default set?

Technical question:

My plugin might or might not need SCN_MODIFIED events (including SC_MOD_BEFOREDELETE). Must NPPM_ADDSCNMODIFIEDFLAGS be sent in NPPN_READY, or can I wait and send it only if and when the user does something which enables the feature (elastic tabstops) that makes use of these events? I understand that once any plugin enables them, they will remain enabled for the entire session. I’m just wondering if I can avoid imposing the performance loss you documented on users who have Columns++ installed but don’t always (or perhaps never) use the elastic tabstops feature.

I’m a lot slower at getting things changed than you are… but my eventual goal now will be to eliminate using SC_MOD_BEFOREDELETE. (In my implementation of elastic tabstops, it has always been a work-around for a common case which exposes an underlying weakness in my design.) In the meantime I’d like to avoid making users pay a performance penalty, relative to unaugmented Notepad++, just for having the plugin installed.

donho

@Coises said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

A small point, but the comments here still use adding SC_MOD_DELETETEXT and SC_MOD_INSERTTEXT as an example, when they are already part of the default. I think you meant to use SC_MOD_BEFOREDELETE and SC_MOD_BEFOREINSERT as examples.

Fixed. Thank you for pointing out this issue.

Also — and I understand you might not agree — would it not be better, as a matter of “future-proofing,” to illustrate best practice as explicitly enabling all SCN_MODIFIED events the plugin plans to use, even if they are (currently) part of the default set?

What do you mean exactely “to illustrate best practice as explicitly enabling all SCN_MODIFIED events the plugin plans to use”?

My plugin might or might not need SCN_MODIFIED events (including SC_MOD_BEFOREDELETE). Must NPPM_ADDSCNMODIFIEDFLAGS be sent in NPPN_READY, or can I wait and send it only if and when the user does something which enables the feature (elastic tabstops) that makes use of these events? I understand that once any plugin enables them, they will remain enabled for the entire session. I’m just wondering if I can avoid imposing the performance loss you documented on users who have Columns++ installed but don’t always (or perhaps never) use the elastic tabstops feature.

I just modified the code for your specific requirement (and it’s a good one):
https://github.com/notepad-plus-plus/notepad-plus-plus/commit/d888fb5f1263f5ea036c610b6980e5c4381ce7eb

So yes, you can send NPPM_ADDSCNMODIFIEDFLAGS anytime you want now - even if you want to set the different events separately at the different time, according users’ need.

Coises

@donho said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

What do you mean exactely “to illustrate best practice as explicitly enabling all SCN_MODIFIED events the plugin plans to use”?

I just mean, should examples and documentation suggest that if a plugin uses SC_MOD_INSERT (which is part of the default) and SC_MOD_BEFOREDELETE (which isn’t), it should send:
SC_MOD_INSERT | SC_MOD_BEFOREDELETE
and not just:
SC_MOD_BEFOREDELETE
so as not to assume SC_MOD_INSERT will always be part of the default? I understand that you might be so certain that the default will not change in the future that there is no point to this.

Should a plugin care what the default mask is? I mention it because I personally don’t see any advantage to plugins being dependent on the default; it doesn’t hurt anything to specify every event that will be used. Once we give plugins the responsibility to declare which events they need enabled, why not routinely declare all of them instead of relying on a particular set of defaults?

So yes, you can send NPPM_ADDSCNMODIFIEDFLAGS anytime you want now - even if you want to set the different events separately at the different time, according users’ need.

Thank you. That makes me feel better about using this message (though I still hope to find time to re-design my way around it eventually).

donho

@Coises said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

Should a plugin care what the default mask is? I mention it because I personally don’t see any advantage to plugins being dependent on the default; it doesn’t hurt anything to specify every event that will be used. Once we give plugins the responsibility to declare which events they need enabled, why not routinely declare all of them instead of relying on a particular set of defaults?

you’re not wrong about this point.

However, the authors of concerning plugins won’t understand why SCN_MODIFIED works with their plugins work under v8.7.5 & previous versions but not under v8.7.6 & later version.
So the 5 events used by Notepad++ are listed to address to the regression for the historic reason,

With the example you provided, plugins use SC_MOD_INSERT | SC_MOD_BEFOREDELETE or only SC_MOD_BEFOREDELETE will get the same effect. I don’t see any inconvenience about it.

Of course, you’re encouraged to improve the document of this message to make your point clearer, if it’s not too redundant.

Eric Pendergrass

I noticed that after I make 1 change to a file, it starts an endless loop of auto-backup every 7 seconds which triggers my onedrive sync and keeps me steadily using high CPU as a result of constantly writing backup files even though I’ve made no changes.

I don’t know what SCN_MODIFIED is or how it works in NP++, but I’m willing to bet I have a bug with a plugin or something that is constantly triggering false changes after my 1st actual change.

I’m going to have to work around and write the file somewhere else as well as decrease how often the files get backed up, but sharing in case this helps anyone else realize the impact of this.

Reach out to me if anyone needs help reproducing this bug or wants to collect any logs from me. Thanks ya’ll!

Mark Olson

@Eric-Pendergrass

I’m willing to bet I have a bug with a plugin or something that is constantly triggering false changes after my 1st actual change.

Have you tried, for each plugin you have installed, temporarily disabling it and seeing if that solves the problem? That usually helps isolate the issue.

I don’t know what SCN_MODIFIED is or how it works in NP++

It’s a notification from Scintilla (the component that actually handles how text is stored and rendered in Notepad++) to plugins telling them “when the text or styling of the document changes or is about to change” (quote from official docs).

Lycan Thrope

@Eric-Pendergrass,
What it seems you’re experiencing is mostly likely the Session Snapshot and periodic backup feature working as designed. This screenshot shows you the default setup:

You can change the time the auto backup occurs or just unselect it, but I suggest you point it somewhere other than your OneDrive directory, because that will include network traffic issues to complete the updates, and that many times is ludicrous for this situation. Better you point it to your local drive as shown in my setup. Hopefully you don’t have a 2 billion size file, or multiples, as that also would explain the horrendous delay. Less is more.

mkupper

@Eric-Pendergrass said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

Reach out to me if anyone needs help reproducing this bug or wants to collect any logs from me

It definitely looks like something that we should attempt to reproduce. Let’s start with your debug info. From Notepad++ click on the ? that is at the top-right of the menu bar at the top of Notepad++ and then select Debug Info.... Click the [Copy debug info into clipboard] button and then paste the results into this forum thread. Next, select the text you just added to the forum’s message box and click the </> button from the forum toolbar that’s just above the message box. This will bracket the selected text with ``` lines.

I believe the main value will be the list of plugins you are using but it’s best to include the full debug info dump.

rdipardo

@mkupper said in New API to fix eventual regression regarding SCN_MODIFIED for some plugins:

It definitely looks like something that we should attempt to reproduce.

I doubt that. The only way a plugin could misuse SCN_MODIFIED would be to program a callback that caused another SCN_MODIFIED event to fire, recursively. Like, say the modificationType field of the scn notification header contained the SC_MOD_DELETETEXT flag, and the callback took the header’s text field and put it back into the document with ::SendMessage(scn->nmhdr.idFrom, SCI_INSERTTEXT, scn->position, scn->text), or something equally stupid.

In other words, a plugin would have to deliberately behave like malware to create such a performance-degrading feedback loop; changing the default event mask as happened in v8.7.7 would not have that effect.

mkupper

@rdipardo I have a bit more than casual interest in what @Eric-Pendergrass reported as I too have seen some unexpected behavior. A couple of weeks ago I noticed that Npp seemed to be writing to disk every 7 seconds despite Notepad++ being idle/unused at the time. However, by the time I had a chance to dig into what was happening the problem went away.

Luckily, it’s happening again to me right now and so I was able to track down what is happening but don’t understand why. I have also confirmed that the issue is not a recent regression and is not related to the SCN_MODIFIED changes. We likely should shift this topic drift to a new forum thread.

However, Notepad++'s behavior still bothers me or at least it is unexpected. What I am seeing is that if a file/tab has been dirtied since I started Notepad++.exe that Notepad++ starts writing that file to the backup folder every 7 seconds when it is the active tab. This occurs with npp versions v8.7.5, v8.7.6, and v8.7.7. This still occurs when Notepad++.exe is started with -noPlugin

@Lycan-Thrope says this is “by design” but it still surprises me. Why does Notepad++ need to flush the current tab’s file to disk every 7 seconds even when Notepad++ is sitting idle in the background? I can stop the continuous flushing either by exiting and restarting Notepad++, by switching to a tab/file that has not been modified since I started Notepad++.exe, or by saving the current tab to disk via Ctrl+S.

If a file was already using the npp/backup copy (red tab) when I started Notepad++.exe then Notepad++ does not flush the file(s) to disk every 7 seconds. The continuous every-7-seconds flushing starts as soon as I type a single letter into a file. It does does not stop when I use Ctrl+Z undo to restore the file back to the state it was in when I started Notepad++.exe.

@Eric-Pendergrass likely has his Notepad++\backup folder included the things monitored by OneDrive as he wants his off-machine backup to stay up to date. Normally we’d give a person an all-thumbs-up for doing that.

@Eric-Pendergrass - you can work around the issue by switching to another file/tab in Notepad++ that has not had any changes made to it since you started Notepad++.exe. If the current active tab has had any changes made to it since you started Notepad++.exe then Notepad++ writes it to disk every 7 seconds meaning your OneDrive updates the Internet based copy of the file even though you are not making any changes.

Alan Kilborn

@mkupper said:

What I am seeing is that if a file/tab has been dirtied since I started Notepad++.exe that Notepad++ starts writing that file to the backup folder every 7 seconds when it is the active tab

I can confirm this behavior with 8.7.6 and 8.7.7; I didn’t try any other versions
I would add to @mkupper’s description: “…and there is no need to write the file because the data has not changed”.
@mkupper Are you going to put in an “issue” on github about this behavior?

Lycan Thrope

@Alan-Kilborn ,
I’m just not sure if it really is an issue. It seems, according to @mkupper solutions, it would seem that on selection and change of a tab, the session 7 second back up kicks in…happening every 7 seconds. Once you change tabs, or save the document, I’m guessing, it flushes the signal to back up that particular document, so…If I use logic on this scenario, it appears it’s working as designed, unless you change the value of back up frequency, or you turn if off, at which point, you now don’t have snapshot backup. Double-edged sword.