Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2

PeterJones

Plugin Authors:

Please note: per this commit, the
NPPM_GETOPENFILENAMES, NPPM_GETOPENFILENAMESPRIMARY & NPPM_GETOPENFILENAMESSECOND messages are being deprecated. They will still be in the codebase for some versions, but will have _DEPRECATED at the end of the constant names in Notepad_plus_msgs.h – and they will be going away eventually.

Don has updated the pluginDemo’s getFileNamesDemo() to show an alternate method to access the filenames that used to be obtained from NPPM_GETOPENFILENAMES, and I am placing those side-by-side here:

Here is the version compatible with v8.8.2-and-forward, for easy copy/paste:

void getFileNamesDemo()
{
	int nbMainViewFile = (int)::SendMessage(nppData._nppHandle, NPPM_GETNBOPENFILES, 0, PRIMARY_VIEW);
	int nbSubViewFile = (int)::SendMessage(nppData._nppHandle, NPPM_GETNBOPENFILES, 0, SECOND_VIEW);
	int nbFile = nbMainViewFile + nbSubViewFile;
	wchar_t toto[10];
	::MessageBox(nppData._nppHandle, generic_itoa(nbFile, toto, 10), L"nb opened files", MB_OK);
	
	wchar_t **fileNames = (wchar_t **)new wchar_t*[nbFile];

	int i = 0;
	for (; i < nbMainViewFile; )
	{
		LRESULT bufferId = ::SendMessage(nppData._nppHandle, NPPM_GETBUFFERIDFROMPOS, i, MAIN_VIEW);
		LRESULT len = ::SendMessage(nppData._nppHandle, NPPM_GETFULLPATHFROMBUFFERID, bufferId, (WPARAM)nullptr);
		fileNames[i] = new wchar_t[len + 1];
		::SendMessage(nppData._nppHandle, NPPM_GETFULLPATHFROMBUFFERID, bufferId, (WPARAM)fileNames[i]);
		++i;
	}

	
	for (int j = 0; j < nbSubViewFile; ++j)
	{
		LRESULT bufferId = ::SendMessage(nppData._nppHandle, NPPM_GETBUFFERIDFROMPOS, j, SUB_VIEW);
		LRESULT len = ::SendMessage(nppData._nppHandle, NPPM_GETFULLPATHFROMBUFFERID, bufferId, (WPARAM)nullptr);
		fileNames[i] = new wchar_t[len + 1];
		::SendMessage(nppData._nppHandle, NPPM_GETFULLPATHFROMBUFFERID, bufferId, (WPARAM)fileNames[i]);
		++i;
	}

	for (int k = 0 ; k < nbFile ; k++)
		::MessageBox(nppData._nppHandle, fileNames[k], L"", MB_OK);




	for (int k = 0 ; k < nbFile ; k++)
	{
		delete [] fileNames[k];
	}
	delete [] fileNames;
}

It is recommended that you make analagous changes in any plugins that use one or more of those three messages as soon as possible.

Mark Olson

@PeterJones
While I’m glad that these messages are being deprecated, I hope they’re not removed altogether! Considering that the reason I asked for the messages to be deprecated in the first place was my own misunderstanding of the interface, I don’t really think the messages are dangerous enough to warrant removal.
I’ll say the same on the GitHub issue as well.

PeterJones

@Mark-Olson said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

While I’m glad that these messages are being deprecated, I hope they’re not removed altogether

To be “deprecated” means to be marked as “do not use, because it will be going away at some point in the future”. Generally, it’s polite to leave deprecated interfaces available for at least a few versions, but they generally do get completely removed eventually. Essentially, “it’s going away soon, but not quite yet”.

rdipardo

@Mark-Olson said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

I don’t really think the messages are dangerous enough to warrant removal.

IIRC, the underlying issue was that @kbilsted’s original implementation of ClikeStringArray was not Unicode-aware.

In other words, an accident waiting to happen, like much of the code in the old template project. Decisions about the API should be guided by safety, not preserving backward compatibility with really old stuff.

Coises

@Mark-Olson said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

@PeterJones
While I’m glad that these messages are being deprecated, I hope they’re not removed altogether! Considering that the reason I asked for the messages to be deprecated in the first place was my own misunderstanding of the interface, I don’t really think the messages are dangerous enough to warrant removal.

I agree. I now regret that I wrote a message that probably helped draw attention to something which I now believe should have been ignored. Both of the concerns which motivated that message turned out to be ill-founded.

Microsoft’s documentation for lstrcpy says, “Warning Do not use.” On further examination, I believe they are being overly dramatic. It is possible to misuse that function, but it is not inherently more problematic than any function that accomplishes the same thing with C-style strings; in the context in question, it is arguably better than the alternatives. I note that despite the warning, the function is not deprecated.

My other concern was that MAX_PATH is no longer a hard limitation in Windows. This interface would be completely broken for that reason — but Notepad++ does not open file paths which exceed MAX_PATH on any version of Windows. So this only becomes a problem if that is changed someday.

If it were up to me, I wouldn’t even deprecate these functions. (Note that NPPM_GETSESSIONFILES has exactly the same weaknesses, but it is not being deprecated, because there is no alternate way to accomplish the same thing.) Deprecating them suggests that removing them in the future would be acceptable, and I see no reason to break working plugins just because they’re not constantly maintained. I would just put a note in the documentation that new code is advised to use the alternate method. These messages do not pose any unique danger if they are used correctly — which, presumably, they are in older plugins, or the errors would have been discovered.

The problem with these functions was clarity of documentation — which has been fixed — and that they are perhaps a little more error-prone than average. However, C interfaces are inherently susceptible to programmer error. C and C++ programmers are used to the pitfalls (Windows is all C interface, even in C++), but for someone trying to bridge between C# (or, I presume, most other languages) and the C interface, it’s easy to miss something.

The error-prone part of this interface is that if you supply inadequate buffers, the mistake can easily go undetected (in this case, until a file path is longer than 130 characters) and then show up in an apparently unrelated part of the program. Interface design can mitigate that sort of thing, but it’s impractical or impossible to eliminate it.

rdipardo

@Coises said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

Note that NPPM_GETSESSIONFILES has exactly the same weaknesses, but it is not being deprecated, because there is no alternate way to accomplish the same thing.

The alternative is to parse session.xml using an XML DOM API, such as many programming languages include in their standard libraries, as was being done for nativeLang.xml before NPPM_GETNATIVELANGFILENAME existed.

The problem is that each alternative would be sui generis to the given plugin’s software ecosystem; even the canonical C++ example would have to take a dependency on TinyXML (or so I fear, since migrating to the more capable TinyXML‍2 is still a long way off).

PeterJones

@rdipardo said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

even the canonical C++ example would have to take a dependency on TinyXML (or so I fear, since migrating to the more capable TinyXML‍2 is still a long way off ).

Correct me if I’m wrong, but the “canonical C++ example” would be able to use TinyXML2 even if N++ itself is still using TinyXML, since the two are independent codebases. Right?

But I agree in principle: making plugindemo require another library – whether tinyxml-old or tinyxml2 or whatever – seems counter to the plugindemo purpose.

rdipardo

@PeterJones said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

Correct me if I’m wrong, but the “canonical C++ example” would be able to use TinyXML2 even if N++ itself is still using TinyXML, since the two are independent codebases. Right?

Not much difference between them, except that version 2 is Unicode-aware. In fact HTML Tag is built with TinyXML2 in order to provide menu localization for pre-8.7 Notepad++ versions. It was even compatible with Windows XP, before I had to drop support for it in the 1.5.2 release.

Vitalii Dovgan

What about the backward compatibility?
Let’s imagine:

One makes these changes to a plugin.
Right after that, this updated version of the plugin requires at least Notepad++ 8.8.2 and will not work with previous versions of Notepad++.
Alternatively, each plugin developer must still keep NPPM_GETOPENFILENAMES for earlier versions of Notepad++ and also add the new code for Notepad++ 8.8.2 and above.

rdipardo

@Vitalii-Dovgan said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

Let’s imagine:

One makes these changes to a plugin.

Right after that, this updated version of the plugin requires at least Notepad++ 8.8.2 and will not work with previous versions of Notepad++.

Not so. The message codes have simply been renamed with a new *_DEPRECATED suffix. No internal code has changed yet. The Notepad++ message handler doesn’t care what name the code maps to, as long as the pre-processor replaces it with the same value when doing macro expansion. So older plugins will continue to work as long as NPPM_GETOPENFILENAMES_DEPRECATED == NPPM_GETOPENFILENAMES == 2032.

By the time Don decides to stop handling message code 2032, even more breaking changes could appear, sending unmaintained plugins into obsolescence anyway.

Coises

@Vitalii-Dovgan said in Announcement: NPPM_GETOPENFILENAMES and related are being deprecated in v8.8.2:

What about the backward compatibility?
Let’s imagine:

One makes these changes to a plugin.

Right after that, this updated version of the plugin requires at least Notepad++ 8.8.2 and will not work with previous versions of Notepad++.
Alternatively, each plugin developer must still keep NPPM_GETOPENFILENAMES for earlier versions of Notepad++ and also add the new code for Notepad++ 8.8.2 and above.

A valid concern, but it doesn’t apply in this case. The reason for that lies in the details:

Existing plugins will continue to work as the message is only being deprecated, not removed; binary compatibility is not affected.

New plugins will still work with older versions of Notepad++, as the recommended replacement for this message is a code sequence using messages that were already available; no new functions are being added. The NPPM_GETOPENFILENAMES* messages were a way to get open file names with less code, and that’s what’s being deprecated. The “long way” hasn’t changed; it’s just been decided that the increased risk of error when using the “short way” isn’t justified to save a few lines of plugin code, so new development should not use it.

What will be affected is the backward compatibility of source when updating to the new header files. If you have a working plugin that uses NPPM_GETOPENFILENAMES, NPPM_GETOPENFILENAMESPRIMARY and/or NPPM_GETOPENFILENAMESSECOND and you update the Notepad_plus_msgs.h header, the constants for those message identifiers will no longer be defined. (They are, instead, defined for the same identifiers with the suffix _DEPRECATED.)

So only plugin authors who need to update the list of messages (which would be only if they are using a new feature, which of course would not be backward compatible) will be affected at all; and they can either work around it by adding the _DEPRECATED suffix or by replacing the code that uses those messages with the recommended alternative.