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

[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:

1. One makes these changes to a plugin.
2. 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:

1. One makes these changes to a plugin.
2. 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:

1. One makes these changes to a plugin.
2. 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.

[Vitalii Dovgan]

Thank you for your clarifications!
I somehow overlooked the messages (such as NPPM_GETFULLPATHFROMBUFFERID) that existed for years!

I have a new question. What about NPPM_GETFULLCURRENTPATH and so on that still implicitly rely on a buffer with a size of MAX_PATH? Shouldn’t they be updated to behave similarly to NPPM_GETFULLPATHFROMBUFFERID ?

[Coises]

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

I have a new question. What about NPPM_GETFULLCURRENTPATH and so on that still implicitly rely on a buffer with a size of MAX_PATH? Shouldn’t they be updated to behave similarly to NPPM_GETFULLPATHFROMBUFFERID ?

For my personal opinion, see my earlier post in this thread.

After discussion, the maintainer of this project decided that deprecation of these messages was warranted. If you have the attention span to go through that entire discussion, and this related one, you’ll see that there was no really satisfactory solution. It’s a C interface. It’s always going to possible to bungle it, and the structure of Windows means that with a very open and flexible plugin interface, like the one in Notepad++, it’s not always possible to defend against such bungling. It’s just a question of preferring less error-prone interfaces to more error-prone ones.

[Vitalii Dovgan]

@Coises
Thank you, I understand the reasoning behind marking NPPM_GETOPENFILENAMES as deprecated. I’m updating NppExec to not use NPPM_GETOPENFILENAMES.

My last question was rather related to NPPM_GETFULLCURRENTPATH, NPPM_GETCURRENTDIRECTORY and the other messages from the same family. These messages:

1. are internally limited by MAX_PATH;
2. do not provide ability to request/retrieve the length required for the returned path/name.

These two issues seem to look very close to the ones resolved by the usage of NPPM_GETFULLPATHFROMBUFFERID instead of NPPM_GETOPENFILENAMES.
So, to me, it looks completely logical to consider the following changes as well:

1. use PathCchRemoveFileSpec for NPPM_GETCURRENTDIRECTORY and so on to remove the internal limit of MAX_PATH;
2. provide the ability to call NPPM_GETFULLCURRENTPATH(0, NULL) to request/retrieve the length required for the returned path. The same with NPPM_GETCURRENTDIRECTORY and so on.

[Coises]

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

My last question was rather related to NPPM_GETFULLCURRENTPATH, NPPM_GETCURRENTDIRECTORY and the other messages from the same family. These messages:

1. are internally limited by MAX_PATH;
2. do not provide ability to request/retrieve the length required for the returned path/name.

See the first part of this comment and the first part of the reply by the maintainer of Notepad++.

The MAX_PATH limitation is present in the file handling routines of Notepad++, not just the interface for these messages. As I read that reply, Don has rejected (for now, at least) consideration that the MAX_PATH constraint might someday be removed. If you know MAX_PATH is enough, there’s no need to request the actual length ahead of time.

I agree that there is some inconsistency in what is considered appropriate for deprecation, but I return to the observation that it will always be possible to screw up calling a C interface, and there were and are compelling reasons to keep the plugin interface as a pure C interface. (If we could pass a std::wstring or a std::vector<std::wstring> the whole problem would go away.) It’s a judgement call what is “too error-prone.” Not worth all that much debate.