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

rpsl_explorer FileMonitor does not notify on file change #33

Open
ChemistAion opened this issue May 28, 2023 · 4 comments
Open

rpsl_explorer FileMonitor does not notify on file change #33

ChemistAion opened this issue May 28, 2023 · 4 comments

Comments

@ChemistAion
Copy link

ChemistAion commented May 28, 2023

There is a missing SHCNRF_ShellLevel flag here - in the type of events for which to receive notifications:

ULONG uId = SHChangeNotifyRegister(hWndListener,
SHCNRF_InterruptLevel | SHCNRF_NewDelivery,
SHCNE_RENAMEITEM | SHCNE_DELETE | SHCNE_UPDATEITEM,
UM_FILE_CHANGED,
1,
&notifyEntry);

It is necessary to include it to capture changes made, e.g. from text-editors (considered as a ShellLevel source).


Other, minor things:

  • UM_FILE_CHANGED should be type of UINT;
  • result of ILCreateFromPathA is not checked (in case of 0);
  • registered-to-notify file is not placed into m_registerIDs - so it would be wrongly considered again

Please find my snipped with corrected version:

static constexpr UINT UM_FILE_CHANGED = WM_USER + 4097;

bool BeginWatch(HWND hWndListener, const std::string& fileName)
{
    auto iter = m_registerIDs.find(fileName);
    
    if (iter != m_registerIDs.end())
    {
        return false;
    }

    PCIDLIST_ABSOLUTE pidl = ILCreateFromPathA(fileName.c_str());

    if (pidl == 0)
    {
        return false;
    }

    SHChangeNotifyEntry notifyEntry = { pidl, FALSE };

    ULONG uId = SHChangeNotifyRegister
    (
        hWndListener,
        SHCNRF_InterruptLevel | SHCNRF_ShellLevel | SHCNRF_NewDelivery,
        SHCNE_RENAMEITEM | SHCNE_DELETE | SHCNE_UPDATEITEM,
        UM_FILE_CHANGED,
        1,
        &notifyEntry
    );

    if (uId != 0)
    {
        m_registerIDs.emplace(fileName, uId);
    }

    return (uId != 0);
}
@FlorianHerickAMD
Copy link
Collaborator

Which text-editors did you experience this with? For example, updates from Notepad++ seem to work just fine on my side.

@ChemistAion
Copy link
Author

ChemistAion commented May 30, 2023

@FlorianHerickAMD:
Latest Notepad++ v8.4.9 x64 on Windows 10 Pro for Workstations (10.0.19045 Build 19045)

Additionally, I have conducted a second test on a different machine, and the results align with the previous findings.
Please see the comment below.

@ChemistAion
Copy link
Author

ChemistAion commented May 30, 2023

@FlorianHerickAMD:
I can also confirm this by testing with the latest Notepad++ v8.4.9 x64 on Windows 11 Enterprise (10.0.22621 Build 22621).
RPS from fresh repo clone, builded with latest VS Community 2022 (17.6.2).

Scenario to reproduce:

  • from rpsl_explorer I have opened `examples\hello_triangle.rpsl';
  • this trigers compilation/reloads jit-module etc.;
  • the same file is opened from Notepad++;
  • making/saving changes does not retrigers jit-module;
  • removing tmp cache-dir during these tryouts also makes no difference;
  • note: no fancy path for RPS was used, here: d:\_rps;

Root cause: UM_FILE_CHANGED message is not generated due to a missing SHCNRF_ShellLevel flag in SHChangeNotifyRegister(...) - as described in the first comment.

@ChemistAion
Copy link
Author

ChemistAion commented May 30, 2023

The issue persists in both Debug and Release configurations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants