microsoft / winfile

Original Windows File Manager (winfile) with enhancements
MIT License
6.75k stars 698 forks source link

Refresh tree on junction or directory symlink creation #412

Closed malxau-msft closed 6 months ago

malxau-msft commented 6 months ago

This PR aims to build upon schinagl's earlier work in #409 and #406 .

In those discussions, the remaining issue was a case where links could be created as hidden, which would cause the left pane to be updated to contain the link (when it should be hidden.)

To summarize, three options were discussed to address it:

  1. Avoid creating junctions/symlinks as hidden.
  2. If they need to be created as hidden, do this before updating the UI, then have the UI check file attributes and decide what to include.
  3. Leave the attribute update where it is today, and have the attribute update also update the UI, meaning that new links would be inserted and then removed when they become hidden.

This PR takes the first approach, because the behavior of Winfile here seems inconsistent with mklink and was unexpected to me. The original file attribute change seemed undesirable, and it triggered later problems for refreshing the tree, so this solves two problems in one.

We still need to ensure junctions aren't included in the left pane if the user is hiding junctions. This PR moves that logic up and out of InsertDirectory, mainly because a previous attempt to add hidden/system handling there showed it would become tricky since callers treat InsertDirectory as a command (and expect a tree node returned.)

Rather than have the FSC handler code inspect file attributes, this PR instead indicates the operation explicitly (mkdir, junction, or directory symlink.) The FSC handler code can be simplified by only applying junction hiding to junction creation. Currently directory creation and directory symlinks are handled identically, so splitting these out was for consistency with junctions.

Previously directory creation had a "quiet" variant, which meant "don't expand the tree." Since this applies to the others too, this PR makes it a flag that can be combined with other operations.

Next steps

While looking into these, a few longstanding issues came to light:

  1. The behavior of creating a junction or directory symbolic link as hidden seems to have originated from copy, where link creation followed copy behavior. The same issue exists with directory copy, where copying a hidden directory will first update the tree view, then update file attributes to mark the new directory as hidden, but leave the hidden directory in the tree. This behavior is longstanding (it exists in the 16 bit versions of Winfile), and unlike here, there's a clear rationale for a copied directory to apply the file attributes of the source.
  2. When file attributes are changed on a directory, marking it hidden/system, the left pane is not updated. This seems to be longstanding behavior. Although this PR doesn't address it, as of this writing, it looks simple to address: the FSC_RENAME handler already inspects file attributes to check if an object is a directory, and based on that, perform left pane updates. Here, we'd want something similar, where if hidden/system files are being hidden and an attribute change occurs on a directory, the operation should either take the current FSC_MKDIR or FSC_RMDIR logic as appropriate. This might be sufficient to address the case above.
  3. An explicit refresh through the Window menu does not refresh the left pane. This appears to be caused by changes in 10.0 in the TC_SETDRIVE handler where if a node is found it is selected without generating a full refresh. I'm not familiar with the history of these changes, so this will require a little more investigation to see what can be done.
schinagl commented 6 months ago

Going for 1) is ok, but basically the solution to a different problem. Overall it is ok

Anyhow: The branch does not compile on my side src\treectl.c(2435,27): error C2065: 'bDirectoryOperation': undeclared identifier

Furthermore there is a warning src\treectl.c(2490,34): warning C4244: '=': conversion from 'LONG_PTR' to 'DWORD', possible loss of data

malxau-msft commented 6 months ago

Sorry, that was bad. Fixed the build error and 64 bit truncation warning.