Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quotes around file paths passed to commands #43

Closed
eeroh opened this issue Apr 4, 2017 · 5 comments
Closed

Add quotes around file paths passed to commands #43

eeroh opened this issue Apr 4, 2017 · 5 comments
Assignees

Comments

@eeroh
Copy link

eeroh commented Apr 4, 2017

Currently the commands do not work properly if there are spaces in the file paths. Please add quotes around the paths that are passed to executed perforce commands.

This has worked in earlier versions, at least in 0.1.8 where the quotes were added to all paths, but the functionality has apparently broken in the refactoring done for the latest 1.0.0 version.

@stef-levesque
Copy link
Owner

Hi @eeroh ,

Quotes are still used for most file path, afaik.
Could you be more specific about which command failed, and what is the file path used ? This may actually be caused by special character, like in issue #38, which commit 0efcd4a should fix.

@eeroh
Copy link
Author

eeroh commented Apr 4, 2017

Hi @stef-levesque, sure, here's an example:

When trying to check out a file for edit in VS Code, the operation prints to the Perforce log rows like

p4 edit /Users/eero/Perforce/eero.ws/Project/Assets/Lib/Scripts/Editor/Object Extensions for Editor/ObjectExtensions.cs

and spaces seem to cause the operation to fail, because for any file in other folders without spaces checking out for edit works.

In this case I'm operating on OS X, and the corresponding command in terminal fails due to not having quotes around the full path, but the same should apply on any operating system.

What's more, by editing /Users/eero/.vscode/extensions/slevesque.perforce-1.0.0/out/src/PerforceCommands.js, it's easy to check that by adding quotes the issue gets fixed.
For example by making a quick hack to function editOpenFile() in the form of

    function editOpenFile() {
        var editor = vscode_1.window.activeTextEditor;
        if (!checkFileSelected()) {
            return false;
        }
        var filePath = editor.document.uri.fsPath;
        //If folder not opened, run p4 in files folder.
        if (checkFolderOpened()) {
            edit(addQuotes(filePath));
        }
        else {
            edit(addQuotes(filePath), addQuotes(Path.dirname(filePath)));
        }
    }
    // HACK: temp utility function
    function addQuotes(str) {
        return (str) ? ('"' + str + '"') : str;
    }

the functionality starts working, and the command in the Perforce log of vs code shows up as

p4 edit "/Users/eero/Perforce/eero.ws/Project/Assets/Lib/Scripts/Editor/Object Extensions for Editor/ObjectExtensions.cs"
//depot/Project/Assets/Lib/Scripts/Editor/Object Extensions for Editor/ObjectExtensions.cs#3 - opened for edit

Now, since it's best to make a neat implementation, I'd really appreciate if you took a look at this to replace my hacks made in a language I hardly use.

Comparing to earlier versions (0.1.8 or 0.1.9), there the code actually did explicitly add the quotes like

function p_editUri(uri) {
	var cmdline = buildCmdline("edit", '"' + uri.fsPath + '"');
...

and so on, but in 1.0.0, following any similar uri.fsPath parameters in PerforceCommands.js shows that the quotes do not get similarly added (not on PerforceService.js side either), so that might have just dropped off in some point.

Anyways, thanks for the quick response, and I hope this clarified the issue.
-Eero

@stef-levesque
Copy link
Owner

Thanks for the detailed explanation, much appreciated 👍

Yes, I see the problem now... All file paths used in P4 command should be surrounded by quote, and ASCII expanded to avoid special character. We already do that in Utils.ts on the scm branch.

Unfortunately, this fix will only be compatible with VSCode March release (v 1.11). See microsoft/vscode#23276 for ETA.

I hope to push a new release over the week-end. In the meanwhile, your local fix seems like a decent workaround for the edit command. Although, addQuotes(...) should be used everywhere we use fsPath. I will try to push an emergency release your fix in.

Thanks again for your help!

stef-levesque added a commit that referenced this issue Apr 5, 2017
@stef-levesque
Copy link
Owner

Fixed. Please update to version 1.1.0

@eeroh
Copy link
Author

eeroh commented Apr 5, 2017

Oh wow, that was fast. Updated and it works 👍 Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants