NPPM_RELOADFILE should return TRUE _only_ on success

Vitalii Dovgan

Due to the recent change around NPPM_RELOADFILE in Notepad++'s code, this question had arisen.
It has been resolved in the scope of the related opened issue, however I’d like to raise it agin in a little bit different context.

As the result of NPPM_DOOPEN, Notepad++ returns either TRUE when the file has been successfully opened or FALSE in case of a failure. And this is the perfect behavior since the caller (originator) of the NPPM_DOOPEN receives a completely clear response that indicates whether the file has actually been opened.
The same applies to NPPM_SWITCHTOFILE.

Now, in case of NPPM_RELOADFILE, relayNppMessages is called, but NPPM_RELOADFILE does not return a bool value to indicate whether the reloading was successful or not.
Imagine a plugin that calls NPPM_RELOADFILE. It gets some resulting value, but this resulting value is irrelevant to the file reloading operation. So the plugin can’t rely on the returning code of NPPM_RELOADFILE and has no ability to identify whether the file has actually been reloaded or not.
So, we need NPPM_RELOADFILE to return TRUE only when the file has actually been reloaded and otherwise to return FALSE.
This is not a question of taste. It’s a question of necessity.

PeterJones

@Vitalii-Dovgan ,

Makes sense to me.

Even the NPPM_RELOADBUFFERID (which does a similar reload, but based on buffer ID instead of file name) returns true/false, so it is definitely strange that NPPM_RELOADFILE always returns true.

I don’t see a reason not to put in a feature request to update NPPM_RELOADFILE’s return value to make it useful.

rdipardo

@Vitalii-Dovgan

See my GitHub comment explaining that this has always been the case.

PluginsManager::relayNppMessages does not return anything; it’s a wrapper around the messageProc plugin API.

If your plugin implements messageProc, the input will be message=NPPM_RELOADFILE, WPARAM={0 (no warning dialog) | 1 (warn)}, LPARAM={file path}. Your plugin’s internal logic can then determine whether or not the file is valid, or even resend NPPM_RELOADFILE with different parameters.

Vitalii Dovgan

I understand that it has always been the case.
What I’m saying now is: this needs to be changed in order to return a meaningful value.
Basically, the change is quite simple:

current code:

BufferID id = MainFileManager.getBufferFromName(longNameFullpath);
if (id != BUFFER_INVALID)
    doReload(id, wParam != 0);

proposed code:

result = FALSE;
BufferID id = MainFileManager.getBufferFromName(longNameFullpath);
if (id != BUFFER_INVALID)
    result = doReload(id, wParam != 0) ? TRUE : FALSE;

donho

Made it correct in https://github.com/notepad-plus-plus/notepad-plus-plus/commit/060396c6988e278dbc10208099010947e72be6dc