Detect paths

[JanCaha]

Some work for #133.

This fixes paths detection functions.

There was one issue with the design though. If the functions is_windows() and is_macos() are not exported, then using function qgis_detect_windows_paths() and qgis_detect_macos_paths() is problematic, as there is no way to avoid running them on Linux (or another platform) without getting error. My solution is to have all the function private for the package and create a public function qgis_detect_paths(). This functions detects platform and uses correct function for paths and it never ends with error. This makes more sense for public use IMO.

[florisvdh]

Hi @JanCaha, thanks for having a look!

using function qgis_detect_windows_paths() and qgis_detect_macos_paths() is problematic, as there is no way to avoid running them on Linux (or another platform) without getting error

Maybe I don't understand what you mean, but IMO the errors are as expected: those functions are not for use in another OS than the one they are meant for; the error is explicit about that.

If the functions is_windows() and is_macos() are not exported

It shouldn't matter for qgis_detect_*() whether the helpers is_windows() and is_macos() are exported or not – but indeed in #133 it is suggested to make them internal. The package tests have access to internal functions and they run is_windows() and is_macos() as conditions.

Regardless of this, one could indeed argue that detecting available paths using qgis_detect_windows_paths() and qgis_detect_macos_paths() is not an essential part of the package (hence could be made internal) since the user can already get the output of these two functions as messages, by running qgis_path(query = TRUE, quiet = FALSE). The latter calls qgis_detect_windows_paths() or qgis_detect_macos_paths() in Windows and macOS respectively and picks exactly one path, after testing that qgis_process is responsive on this path (and in Linux it only supports qgis_process in the PATH except when preset to another path, as in each OS). But I think it's still a bonus to have one or more exported qgis_detect_*() functions that just enlist (untested but available) qgis_process installations without further testing and selection.

[florisvdh]

Your cross-platform detect-function currently enlists all Windows or macOS paths that are verified to exist (without actually testing yet whether they can run), i.e. what the existing Windows or macOS functions did, while on Linux it just returns qgis_process without its full path and without verification of its existence. I think for the Linux side it should get at the same level for this function to have added value in Linux and achieve true cross-platform usage, i.e. verifying the file's existence and returning the full path, and better still testing several possible locations regardless of PATH. For the PATH's qgis_process in Linux you could call system("which qgis_process", intern = TRUE) and testing whether it has >0 characters. Still, this is not the same as the Windows and macOS approach, where multiple possible locations are tested just for there existence. So just returning the qgis_process filepath for the first hit on the system's PATH will miss possible other locations (not so for win and mac). That would require a dedicated qgis_detect_linux_paths() function that tests various possible locations.

So I suggest either to add that for Linux and use this in the overall detect function, or to exclude other OSes than Windows + macOS (error telling simply that other OSes are not supported by the function), or to just stick to the two current exported functions for Windows + macOS. I had no further intentions than the latter, but you're welcome to extend for Linux.

[florisvdh]

Hi @JanCaha I cherry-picked one of your fixes in #153 (f8fbd37). I also renamed the windows and the macos functions – as said I had no further plans on this topic but I'm open to extending this!

Please let me know whether you'd like to extend this for detecting possible Linux paths (or just exclude Linux) and proceed to create a cross-platform qgis_detect_paths() function.

[florisvdh]

Closing for now; feel free to re-open @JanCaha if you'd like to extend further.

[florisvdh]

From #152 (comment):

, or to exclude other OSes than Windows + macOS (error telling simply that other OSes are not supported by the function)

This way has been implemented in a1f4b5b!