-
Notifications
You must be signed in to change notification settings - Fork 6
fix(RemoteTreeBrowser): add sh -c prefix (#65) #66
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,4 +228,41 @@ end | |
| -- Expose is_localhost for other modules that might need it | ||
| M.is_localhost = is_localhost | ||
|
|
||
| -- Shell script for listing directory contents (sorted) | ||
| -- Uses sh -c to ensure POSIX shell compatibility (works with fish, zsh, etc.) | ||
| -- Uses IFS= read -r to handle filenames with spaces/backslashes | ||
| local LIST_DIR_SCRIPT = [[ | ||
| cd "$1" && find . -maxdepth 1 | sort | while IFS= read -r f; do | ||
|
||
| if [ "$f" != "." ]; then | ||
| if [ -d "$f" ]; then | ||
| echo "d ${f#./}" | ||
| else | ||
| echo "f ${f#./}" | ||
| fi | ||
| fi | ||
| done | ||
| ]] | ||
|
|
||
| -- Shell script for listing directory contents (unsorted, with error suppression) | ||
| local LIST_DIR_SCRIPT_UNSORTED = [[ | ||
| cd "$1" 2>/dev/null && find . -maxdepth 1 -not -name "." | while IFS= read -r f; do | ||
|
||
| if [ -d "$f" ]; then | ||
| echo "d ${f#./}" | ||
| else | ||
| echo "f ${f#./}" | ||
| fi | ||
| done | ||
| ]] | ||
|
|
||
| --- Build a shell command for listing directory contents via SSH | ||
| --- @param path string The remote directory path | ||
| --- @param opts? {sorted?: boolean} Options: sorted (default true) | ||
| --- @return string The shell command to execute | ||
| function M.build_list_dir_cmd(path, opts) | ||
|
||
| opts = opts or {} | ||
| local sorted = opts.sorted ~= false -- default to true | ||
| local script = sorted and LIST_DIR_SCRIPT or LIST_DIR_SCRIPT_UNSORTED | ||
|
Comment on lines
+257
to
+264
|
||
| return string.format("sh -c %s _ %s", vim.fn.shellescape(script), vim.fn.shellescape(path)) | ||
| end | ||
|
|
||
| return M | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,71 +1,55 @@ | ||
| -- Test SSH command escaping functionality | ||
| local test = require("tests.init") | ||
| local ssh_utils = require("async-remote-write.ssh_utils") | ||
|
|
||
| test.describe("SSH Command Escaping", function() | ||
| test.it("should properly escape paths with spaces", function() | ||
| local path = "/home/user/My Documents/test dir/" | ||
| local escaped_path = vim.fn.shellescape(path, 1) | ||
|
|
||
| -- Test that the escaped path contains proper quoting | ||
| test.assert.contains(escaped_path, "My Documents", "Escaped path should contain the directory name") | ||
|
|
||
| -- Test SSH command construction | ||
| local ssh_cmd = string.format( | ||
| 'cd %s && find . -maxdepth 1 | sort | while read f; do if [ \\"\\$f\\" != \\".\\" ]; then if [ -d \\"\\$f\\" ]; then echo \\"d \\${f#./}\\"; else echo \\"f \\${f#./}\\"; fi; fi; done', | ||
| escaped_path | ||
| ) | ||
| local ssh_cmd = ssh_utils.build_list_dir_cmd(path) | ||
|
|
||
| -- Verify command structure | ||
| test.assert.contains(ssh_cmd, "cd", "SSH command should contain cd command") | ||
| test.assert.contains(ssh_cmd, "sh -c", "SSH command should use sh -c") | ||
| test.assert.contains(ssh_cmd, "find", "SSH command should contain find command") | ||
| test.assert.contains(ssh_cmd, '\\"\\$f\\"', "SSH command should have properly escaped variables") | ||
| test.assert.contains(ssh_cmd, "My Documents", "SSH command should contain the path") | ||
| end) | ||
|
|
||
| test.it("should properly escape paths with quotes", function() | ||
| local path = "/home/user/test's dir/" | ||
| local escaped_path = vim.fn.shellescape(path, 1) | ||
|
|
||
| -- Test SSH command construction with quotes | ||
| local ssh_cmd = string.format( | ||
| 'cd %s && find . -maxdepth 1 | sort | while read f; do if [ \\"\\$f\\" != \\".\\" ]; then if [ -d \\"\\$f\\" ]; then echo \\"d \\${f#./}\\"; else echo \\"f \\${f#./}\\"; fi; fi; done', | ||
| escaped_path | ||
| ) | ||
| local ssh_cmd = ssh_utils.build_list_dir_cmd(path) | ||
|
|
||
| -- Verify command doesn't break with quotes | ||
| test.assert.contains(ssh_cmd, "cd", "SSH command should contain cd command") | ||
| test.assert.truthy(#ssh_cmd > 50, "SSH command should be properly constructed") | ||
| -- Verify command structure | ||
| test.assert.contains(ssh_cmd, "sh -c", "SSH command should use sh -c") | ||
| -- Verify the path is properly shell-escaped and passed as argument | ||
| local escaped_path = vim.fn.shellescape(path) | ||
| test.assert.contains(ssh_cmd, escaped_path, "SSH command should contain properly escaped path") | ||
| end) | ||
|
|
||
| test.it("should properly escape paths with special characters", function() | ||
| local path = "/home/user/test (dir) & more/" | ||
| local escaped_path = vim.fn.shellescape(path, 1) | ||
|
|
||
| -- Test SSH command construction with special characters | ||
| local ssh_cmd = string.format( | ||
| 'cd %s && find . -maxdepth 1 | sort | while read f; do if [ \\"\\$f\\" != \\".\\" ]; then if [ -d \\"\\$f\\" ]; then echo \\"d \\${f#./}\\"; else echo \\"f \\${f#./}\\"; fi; fi; done', | ||
| escaped_path | ||
| ) | ||
| local ssh_cmd = ssh_utils.build_list_dir_cmd(path) | ||
|
|
||
| -- Verify command structure is maintained | ||
| test.assert.contains(ssh_cmd, "cd", "SSH command should contain cd command") | ||
| test.assert.contains(ssh_cmd, "sh -c", "SSH command should use sh -c") | ||
| test.assert.contains(ssh_cmd, "find", "SSH command should contain find command") | ||
| test.assert.contains(ssh_cmd, '\\"\\$f\\"', "SSH command should have properly escaped variables") | ||
| end) | ||
|
|
||
| test.it("should handle simple paths without breaking", function() | ||
| local path = "/home/user/simple/" | ||
| local escaped_path = vim.fn.shellescape(path, 1) | ||
|
|
||
| -- Test SSH command construction | ||
| local ssh_cmd = string.format( | ||
| 'cd %s && find . -maxdepth 1 | sort | while read f; do if [ \\"\\$f\\" != \\".\\" ]; then if [ -d \\"\\$f\\" ]; then echo \\"d \\${f#./}\\"; else echo \\"f \\${f#./}\\"; fi; fi; done', | ||
| escaped_path | ||
| ) | ||
| local ssh_cmd = ssh_utils.build_list_dir_cmd(path) | ||
|
|
||
| -- Verify command contains expected elements | ||
| test.assert.contains(ssh_cmd, "/home/user/simple", "SSH command should contain the path") | ||
| test.assert.contains(ssh_cmd, "find . -maxdepth 1", "SSH command should contain find with maxdepth") | ||
| test.assert.contains(ssh_cmd, "sort", "SSH command should contain sort") | ||
| test.assert.contains(ssh_cmd, "while read f", "SSH command should contain while loop") | ||
| test.assert.contains(ssh_cmd, "while IFS= read -r f", "SSH command should contain while loop") | ||
| end) | ||
|
|
||
| test.it("should pass path as argument to avoid quoting issues", function() | ||
| local path = '/home/user/test\'s "quoted" dir/' | ||
| local ssh_cmd = ssh_utils.build_list_dir_cmd(path) | ||
|
|
||
| -- The key feature: path is passed as $1 argument, not embedded in script | ||
| test.assert.contains(ssh_cmd, "_ ", "SSH command should have placeholder for $0") | ||
| test.assert.contains(ssh_cmd, 'cd "$1"', "Script should reference $1 for path") | ||
| end) | ||
| end) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorted = falsecurrently selectsLIST_DIR_SCRIPT_UNSORTED, which removes sorting entirely. In the code being replaced here, the output was sorted (there was a| sort), so this is a behavior change that can affect UI ordering. If the intent is to preserve existing sorted output, callssh_utils.build_list_dir_cmd(path)(default sorted) or pass{ sorted = true }.