Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • C create-react-app
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,547
    • Issues 1,547
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 417
    • Merge requests 417
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Meta
  • create-react-app
  • Merge requests
  • !2282

Add WSL support to launchEditor utility / error overlay functionality

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Administrator requested to merge github/fork/noinkling/master into master May 20, 2017
  • Overview 2
  • Commits 1
  • Pipelines 0
  • Changes 1

Created by: noinkling

In WSL (Bash on Ubuntu on Windows), Windows executables can be launched from within the shell, however setting EDITOR or REACT_EDITOR to a Windows editor and feeding launchEditor() an absolute path currently results in it trying to open a file at e.g. C:\mnt\c\...\foo.js (usually resulting in a new file with that save path), rather than just opening C:\...\foo.js like it should do. Thankfully, the interop functionality can actually translate between the WSL and Windows version of a path (when it's located on the Windows file system), as long as it's provided as a relative path, which is what this change does.

Just to make it clear, these are the possibilities when using this utility under WSL after this change:

Linux-based editor
(e.g. nano)
Windows-based editor
(e.g. VS Code)
File in "Linux" FS (not accessible from Windows) Works (no difference) Doesn't work (no difference). Current WSL behavior is to print "Unable to translate current working directory. Using C:\Users\Whoever" in terminal, which at least gives a hint to users why it won't work.
File in Windows FS (and accessible from within WSL via /mnt/wherever) Works (path will be unnecessarily transformed to relative, but no practical difference) Works now! (most common scenario)

In other words I don't believe there's any sort of regression with this change. I've tested all 4 scenarios with various editors.

Potential improvements

  • In the scenario of Linux FS + Windows editor, it would be nice to simply disallow it and display a custom message, but it depends on detecting that the editor is Windows-based - you could do this by checking extensions and/or using the file command, but even then the Windows versions of VS Code and Atom come with bash script launchers. You could also check where it resolves to (e.g. which $EDITOR), but that comes with some caveats as well (see below).

  • There's a bit of an edge-case where the file could be located under /mnt/, but not on a Windows FS volume, in which case it would be given a relative path but still fail if you tried to open it in a Windows editor. A more accurate, but more complicated way to detect Windows volumes is to check if the filesystem type of the mount is "drvfs" - I can't find an easy way to do this from Node, but you can, for example, run mount | grep /mnt/c or grep /mnt/c /proc/mounts or findmnt -P /mnt/c, then parse it.

Anyway I'm not sure how much you guys care about supporting WSL, but it seems to be gaining popularity for those of us using Windows for one reason or another, so I thought it would at least be worth some consideration.

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: github/fork/noinkling/master