-
Notifications
You must be signed in to change notification settings - Fork 38.9k
./code helper script needs some fixes #137954
Description
Weird decision to use python for path resolution in a bash script when it can all be done locally.
seems this may only be a mac decision too? there's half a dozen code launchers throughout the source tree, but this is the strangest one we could improve a bit: https://github.com/microsoft/vscode/blob/main/resources/darwin/bin/code.sh
Also relying on python means the code command breaks if python isn't available, which doesn't seem ideal (it broke for me when running under a pyenv controlled path before a python version was defined, so python couldn't be used and then code just breaks).
I'd propose a rewrite as:
# Get absolute path of the directory holding the "code" shell script
CONTENTS="$(cd "$(dirname "$0")"; pwd)"
# Jump up 3 more directories to the Contents/ path
for i in {0..2}; do
CONTENTS="$(dirname "$CONTENTS")"
done
# or, could skip the "absolute path resolution" completely and just trust shell path navigation works:
# CONTENTS="$(dirname "$0")/../../../"
ELECTRON="$CONTENTS/MacOS/Electron"
CLI="$CONTENTS/Resources/app/out/cli.js"
ELECTRON_RUN_AS_NODE=1
exec "$ELECTRON" "$CLI" --ms-enable-electron-run-as-node "$@" Alternatively, if for some reason we love python so much we want to re-implement bash primitives using it, a better solution would be:
function contentspath() {
python -c "import pathlib,sys; print(pathlib.Path(sys.argv[1]).resolve().parent.parent.parent.parent)" "$1"
}
CONTENTS="$(contentspath "$0")"
ELECTRON="$CONTENTS/MacOS/Electron"
CLI="$CONTENTS/Resources/app/out/cli.js"
ELECTRON_RUN_AS_NODE=1
exec "$ELECTRON" "$CLI" --ms-enable-electron-run-as-node "$@" also the original code script has a bug where it's giving $0 as an argument to the python script instead of $1 as the actual parameter (but the parameter is always just $0 anyway, so $1 == $0 and the inaccurate argument never has an impact).