Avoid relativizing paths in the project outline

[drok]

In the project outline view, paths for the Makefile and build log were automatically made relative. If the setting was an absolute path, the result would be ugly:

For a makeDirectory setting '/tmp/amhello-debug', the outline would print

Makefile: [../../../tmp/amhello-debug/Makefile]

This was not only unsightly, but also wrong. See https://unix.stackexchange.com/questions/13858/do-the-parent-directorys-permissions-matter-when-accessing-a-subdirectory

In short, if one of the parent directories (..) would be inaccessible, the ../../../tmp/amhello-debug path is wrong because it's inaccessible, even though /tmp/amhello-debug is accessible.

[gcampbell-msft]

@drok To help us confirm that this works, along with our own testing, could you provide a screenshot of this working after your changes? Thanks!

Additionally, can you please make a CHANGELOG entry for this change, following the pattern already set? Yours will go in the 0.8 section, feel free to tag yourself to give yourself credit!

[drok]

@gcampbell-msft

@drok To help us confirm that this works, along with our own testing, could you provide a screenshot of this working after your changes? Thanks!

Certainly:

This PR affects the lines "Makefile:" and "Build Log:" The workspace (source code) is at /home/radu/proj/amhello The "out-of-tree" builddir (makeDirectory) is /tmp/amhello-BB A "buildLog" is used to get around the "infinite --dry-run" bug (#500), and it lives in /tmp/amhello-a/.vscode/build.log2 A "makefilePath" is not specified (thus "Makefile" is a default filename) "after-" images show the effect after applying this patch. "before-" show the behaviour before the patch (at commit 7f27375)

I have highlighted in red (#B22222) the variants I think are undesirable, and in green (#3CB371) the variants I think would be better.

If #500 is merged after this, The display will change as follows:

In this case, the "(not found)" text is wrong, because the Makefile does exist in the makeDirectory. It happens because the outline code (tree.ts) attempts to find the file separately from the getCommandForConfiguration() function, and searches in the wrong locations. I believe getCommandForConfiguration() is and should be the authoritative source for resolving the Makefile location from configuration, and that discovered path should be displayed. The outline is also missing the "makeDirectory", which is essential information, much more so than the discovered make path, which is pointless.

There is more work needed to make the outline display accurate and useful, and some of that work (ie, authoritative resolution of the Makefile) happens in another PR I have not yet proposed: "fix-out-of-tree-builddir"

I have also added CHANGELOG entry, crediting myself as generously sponsored by @Mergesium.

[drok]

@gcampbell-msft my previous update conflicted with CHANGELOG.md from main; the latest forced push resolves the conflict by rebasing the branch onto the fresh main.

[andreeis]

@drok , thank you for the suggestion to include make directory (what is given via -C to "make", right?) into the UI panel. I opened work item https://github.com/microsoft/vscode-makefile-tools/issues/520 to track this idea.

I am interested to find out more about the wrong location of the makefile in the tree. Indeed getCommandForConfiguration() calculates correctly many things but once it does, it stores the latest values into some globals, which are then read so that we don't compute every time what getCommandForConfiguration does. You some some wrong values, I wonder if the bug would be somewhere else, like we forget to call getCommandForConfiguration after some other actions that result in change of those variables.

Indeed, always relativizing was not a good idea but I'm not sure about always not relativizing. The original reason behind the relativizing strategy was to have shorter easier to understand paths. What do you think we would compare the relative versus non relative path and chose the shortest? Or in-tree always relativize and out-of-tree always don't relativize? Additionally, check the permissions situation which I was not aware of. Thank you for explaining that.

[drok]

@andreeis,

What do you think we would compare the relative

The intent with this PR is to make these forms of feedback less jarringly terrible.

However, since you ask, and I appreciate you thinking more in depth about the issue of providing quality feedback to the user, I would say that the outline panel needs some focused effort. Here is how I would present the paths:

The Makefile item

I would not display this item at all, unless:

1. the user's setting explicitly provides an override from the default, or
2. the default resolves ambiguously (eg, both a GNUmakefile and a Makefile exist), ie a different file being used than the user reasonably might expect, or
3. there is no existing makefile.

If a override setting is provided, I would display the text of the setting, unexpanded, eg, it might be ${someOtherVariable}. If the expanded value does not expand to an existing file, I would show a ⚠️ or squiggles. If it does expand correctly, I would put the actual expansion (exactly as expanded, not normalized) in a tooltip, along with a ✔️ mark.

If the file does not exist, I would display the filename that was expected to be found and was not found, and provide a button that might resolve the absence (eg, run the './configure' command in the builddir, or edit a new file for non-autotools projects)

If there is no ambiguity, and no override, it is understood that whatever makefile exists in the $makeDirectory is the relevant one. No "Makefile" line needs to be displayed; no user feedback is needed.

BuildLog item

I would deprecate this feature, because it causes more problems than it solves. (once #500 or alternate is integrated and some configuration can be reliably extracted).

While the feature exists, I would display it in the same way as the Makefile above. Only if set by the user, showing the user's setting, unexpanded, and with a visual indication of whether a file was found or not.

makeDirectory item

Thank you for taking this as a separate https://github.com/microsoft/vscode-makefile-tools/labels/enhancement item in #520.

I would always display this. If not set, I would show the wording "[Workspace]", or similar as long as it's not ambiguous. If ambiguous (eg, two projects in one workspace, and the expansion is a relative name), the same disambiguation that vscode uses (it seems to use the name of the first folder in the workspace, with the leading path removed, see the tooltip shown for the "Workspace" item in the explorer pane of vscode)

If the user has given a setting, same as above, I would show the text of the setting, unexpanded, with a tooltip indicating the expansion, not normalized. It should be understood that if the expansion is a relative directory, it is relative to the workspace. (but how to resolve a relative directory when there are multiple project folders in the same workspace?)

Generally I believe best practice is to show paths as configured by the user, not normalized.

[gcampbell-msft]

@drok I'd like to merge this PR, could you fix the merge conflicts? Thanks!

[drok]

@drok I'd like to merge this PR, could you fix the merge conflicts? Thanks!

No problem.. rebased from b37f0e1 to main, currently at 1635e95, resolves conflict in CHANGELOG.md created at 7ec4385..1635e95