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.

Sign up for GitHub

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

Fix pwsh compatibility bug #3590

Closed
wants to merge 3 commits into from
Closed

Fix pwsh compatibility bug #3590

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d2cefc
Fix pwsh compatibility bug
shishini Nov 28, 2023
c97f5aa
Fix issues flagged by ci/circleci: test-lint
shishini Nov 28, 2023
136ab35
Fix issue flagged by ci/circleci: test-lint
shishini Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions cider.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,21 +791,23 @@ removed, LEIN-PLUGINS, LEIN-MIDDLEWARES and finally PARAMS."
"Removes the duplicates in DEPS."
(cl-delete-duplicates deps :test 'equal))

(defun cider--jack-in-cmd-pwsh-p (command)
"Returns whether COMMAND is pwsh."
(string-equal command "pwsh"))

(defun cider--jack-in-cmd-powershell-p (command)
"Returns whether COMMAND is PowerShell."
(or (string-equal command "powershell")
(string-equal command "pwsh")))
"Returns whether COMMAND is Powershell."
(string-equal command "powershell"))

(defun cider--shell-quote-argument (argument &optional command)
"Quotes ARGUMENT like `shell-quote-argument', suitable for use with COMMAND.

Uses `shell-quote-argument' to quote the ARGUMENT, unless COMMAND is given
and refers to PowerShell, in which case it uses (some limited) PowerShell
rules to quote it."
(if (cider--jack-in-cmd-powershell-p command)
;; please add more PowerShell quoting rules as necessary.
(format "'%s'" (replace-regexp-in-string "\"" "\"\"" argument))
(shell-quote-argument argument)))
(cond ((cider--jack-in-cmd-pwsh-p command) (format "'%s'" argument))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to add some comment explaining the need for the different escaping rules.

Copy link
Contributor Author

@shishini shishini Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its important to clarify that this for me, is workaround for a bug that is either in the clojure cli tool or in powershell
clojure cli should behave the same on both shells, since as per powerhsell docs the quoting rules are the same

I created a question on ask clojure , suggesting that this is a bug in the clojure command line tool, since i cannot replicate this behavior with any other command

Are you ok with documenting this as a workaround, or do your prefer another solution, i think the calva vs code plugin in windows skips the cli tool and use the java command directly , so they dont seem to have this issue

they seem to run this command

; Starting Jack-in Terminal: pushd c:\dev\lang\clojure\project\ukg & java -jar ".calva\deps.clj.jar" -Sdeps "{:deps {nrepl/nrepl {:mvn/version,""1.0.0""},cider/cider-nrepl {:mvn/version,""0.28.5""}}}" -M -m nrepl.cmdline --middleware "[cider.nrepl/cider-middleware]" & popd

for me the calva solution, seem a bit heavy handed, as the plugin on vs code, download his own cli jar files and i dont know what else, i like that cider doesnt install java jars and use the system cli tools

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documenting this as a workaround is fine - e.g. I just learned what this is about and I imagine others reading the codebase will be in the same boat as me. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I thought powershell and pwsh were exactly the same thing)

((cider--jack-in-cmd-powershell-p command) (format "'%s'" (replace-regexp-in-string "\"" "\"\"" argument)))
(t (shell-quote-argument argument))))

(defun cider--powershell-encode-command (cmd-params)
"Base64 encode the powershell command and jack-in CMD-PARAMS for clojure-cli."
Expand Down Expand Up @@ -1625,7 +1627,7 @@ Params is a plist with the following keys (non-exhaustive)
(defun cider--format-cmd (command-resolved command cmd-params)
"Format COMMAND-RESOLVED or COMMAND followed by CMD-PARAMS."
(format "%s %s" command-resolved
(if (cider--jack-in-cmd-powershell-p command)
(if (or (cider--jack-in-cmd-pwsh-p command) (cider--jack-in-cmd-powershell-p command))
(cider--powershell-encode-command cmd-params)
cmd-params)))

Expand Down
Loading