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

Fix #176 - add quotes to command #178

Merged
merged 10 commits into from
Sep 4, 2024
Merged

Conversation

fdodino
Copy link
Contributor

@fdodino fdodino commented Aug 26, 2024

Arregla #176

Le agregué las comillas y probé con esta carpeta en Linux Mint que originalmente me daba error: de es& "mo!

image

image

Algunas cosas

  • voy a ver si lo pruebo en Windows
  • si a una persona se le ocurre poner comillas simples y dobles, algo como soy"muy 'original no hay forma de que funcione
  • por defecto usamos comillas dobles, salvo que en la carpeta haya comillas dobles, ahí usamos comillas simples

@fdodino fdodino requested review from ivojawer and PalumboN August 26, 2024 00:05
Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.09%. Comparing base (f87c5af) to head (5be18c9).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
- Coverage   84.42%   83.09%   -1.33%     
==========================================
  Files          17       17              
  Lines         443      420      -23     
  Branches      101       95       -6     
==========================================
- Hits          374      349      -25     
- Misses         69       71       +2     
Flag Coverage Δ
lsp-ide-client 90.09% <100.00%> (-3.20%) ⬇️
lsp-ide-server 80.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ivojawer ivojawer left a comment

Choose a reason for hiding this comment

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

Buenisimo Fer! 💎 💎 💎

Hay un caso que no me queda claro si querriamos tenerlo en cuenta o no y es si el path de la CLI (suponiendo que no se agrego al PATH) tiene espacios, por ejemplo un comando asi:

C:\Users\Ivan\Mis Documentos\Otra carpeta\wollok-ts-cli.exe repl "miarchivo.wlk" -p "C:\Path a Mi proyecto\"

No se como lo tomarian las shells

@fdodino
Copy link
Contributor Author

fdodino commented Aug 28, 2024

Buenisimo Fer! 💎 💎 💎

Hay un caso que no me queda claro si querriamos tenerlo en cuenta o no y es si el path de la CLI (suponiendo que no se agrego al PATH) tiene espacios, por ejemplo un comando asi:

C:\Users\Ivan\Mis Documentos\Otra carpeta\wollok-ts-cli.exe repl "miarchivo.wlk" -p "C:\Path a Mi proyecto\"

No se como lo tomarian las shells

Qué bueno que se te ocurrió! No había pensado en ese caso. Voy a agregarlo.

@ivojawer
Copy link
Contributor

ivojawer commented Aug 28, 2024

Buenas estuve probando en Windows, traigo resultados interesantes 😬

Ejecutable + Argumentos Con Espacios

Con esta estructura de archivos

.
└── C:/Users/ivoja/Documents/pruebas-wollok/
    ├── mi proyecto wollokero/
    │   ├── package.json
    │   └── src/
    │       └── pepita.wlk
    └── UNA CARPETA CON ESPACIOS/
        └── wollok-ts-cli-win-x64.exe

Powershell

Pareceria que los strings no funcionan como bash y no logra parsear "C:\UN PATH\wollok-ts-cli.exe"

PS C:\Users\ivoja> "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"
At line:1 char:94
+ ... llok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Use ...
+                                                              ~~~~
Unexpected token 'repl' in expression or statement.
    + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : UnexpectedToken

Una googleadita rapida y pareceria que se tiene que escribir con un & adelante 🤌 (esto lo probe powershell directamente asi que no se como funcionara en una task de vscode):

PS C:\Users\ivoja> & "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"
🖥️  Initializing Wollok REPL for file c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

CMD

Aca pasa parecido pero da mas bronca

 *  Executing task: "C:\Users\ivoja\Documents\pruebas-wollok\UNA CARPETA CON ESPACIOS\wollok-ts-cli-win-x64.exe" repl "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero\src\pepita.wlk" --skipValidations --darkMode  -p "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero" 

'C:\Users\ivoja\Documents\pruebas-wollok\UNA' is not recognized as an internal or external command,
operable program or batch file.

 *  The terminal process "C:\Windows\System32\cmd.exe /d /c "C:\Users\ivoja\Documents\pruebas-wollok\UNA CARPETA CON ESPACIOS\wollok-ts-cli-win-x64.exe" repl "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero\src\pepita.wlk" --skipValidations --darkMode  -p "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero"" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it. 

Si copias y pegas el comando en la terminal de command prompt funciona sin ningun problema, porque en realidad el problema es que vscode invoca el ejecutable del cmd asi: C:\Windows\System32\cmd.exe /d /c <aca va el comando>

Y para que funcione con espacios hay que cerrarlo todo con comillas, esto lo probe y funciona en cmd

C:\Users\ivoja>C:\Windows\system32\cmd.exe /d /c ""C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero""
🖥️  Initializing Wollok REPL for file c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

Git Bash

Este me rompio el cora 💔 No logro entender que es lo que esta fallando y no lo pude arreglar para que ande en vscode, solo se que si copias y pegas el comando en Git Bash arranca

 *  Executing task: "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero" 

/usr/bin/bash: -c: line 1: unexpected EOF while looking for matching `"'

 *  The terminal process "C:\Program Files\Git\bin\bash.exe '-c', '"C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"'" terminated with exit code: 2. 
 *  Terminal will be reused by tasks, press any key to close it. 

Solo Argumentos Con Espacios

Con esta estructura de archivos

C:/Users/ivoja/Documents/pruebas-wollok
  mi proyecto wollokero
    package.json
    src
      pepita.wlk
  wollok-ts-cli-win-x64.exe

Reverti el ultimo commit para que al path de CLI no le ponga comillas

Git Bash ✔️

CMD ✔️

Powershell

Falla con las comillas dobles

 *  Executing task: C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero" 

🖥️  Initializing Wollok REPL for file  c:/Users/ivoja/Documents/pruebas-wollok/mi on c:/Users/ivoja/Documents/pruebas-wollok/mi
✗ File c:/Users/ivoja/Documents/pruebas-wollok/mi doesn't exist or is outside of project c:/Users/ivoja/Documents/pruebas-wollok/mi!

 *  The terminal process "C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -Command C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it. 

Le cambie en el codigo para que siempre use simples y arranco

 *  Executing task: C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl 'c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk' --skipValidations --darkMode  -p 'c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero' 

🖥️  Initializing Wollok REPL for file  c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

@fdodino
Copy link
Contributor Author

fdodino commented Aug 28, 2024

Buenas estuve probando en Windows, traigo resultados interesantes 😬

Ejecutable + Argumentos Con Espacios

Con esta estructura de archivos

.
└── C:/Users/ivoja/Documents/pruebas-wollok/
    ├── mi proyecto wollokero/
    │   ├── package.json
    │   └── src/
    │       └── pepita.wlk
    └── UNA CARPETA CON ESPACIOS/
        └── wollok-ts-cli-win-x64.exe

Powershell

Pareceria que los strings no funcionan como bash y no logra parsear "C:\UN PATH\wollok-ts-cli.exe"

PS C:\Users\ivoja> "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"
At line:1 char:94
+ ... llok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Use ...
+                                                              ~~~~
Unexpected token 'repl' in expression or statement.
    + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : UnexpectedToken

Una googleadita rapida y pareceria que se tiene que escribir con un & adelante 🤌 (esto lo probe powershell directamente asi que no se como funcionara en una task de vscode):

PS C:\Users\ivoja> & "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"
🖥️  Initializing Wollok REPL for file c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

CMD

Aca pasa parecido pero da mas bronca

 *  Executing task: "C:\Users\ivoja\Documents\pruebas-wollok\UNA CARPETA CON ESPACIOS\wollok-ts-cli-win-x64.exe" repl "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero\src\pepita.wlk" --skipValidations --darkMode  -p "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero" 

'C:\Users\ivoja\Documents\pruebas-wollok\UNA' is not recognized as an internal or external command,
operable program or batch file.

 *  The terminal process "C:\Windows\System32\cmd.exe /d /c "C:\Users\ivoja\Documents\pruebas-wollok\UNA CARPETA CON ESPACIOS\wollok-ts-cli-win-x64.exe" repl "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero\src\pepita.wlk" --skipValidations --darkMode  -p "c:\Users\ivoja\Documents\pruebas-wollok\mi proyecto wollokero"" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it. 

Si copias y pegas el comando en la terminal de command prompt funciona sin ningun problema, porque en realidad el problema es que vscode invoca el ejecutable del cmd asi: C:\Windows\System32\cmd.exe /d /c <aca va el comando>

Y para que funcione con espacios hay que cerrarlo todo con comillas, esto lo probe y funciona en cmd

C:\Users\ivoja>C:\Windows\system32\cmd.exe /d /c ""C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero""
🖥️  Initializing Wollok REPL for file c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

Git Bash

Este me rompio el cora 💔 No logro entender que es lo que esta fallando y no lo pude arreglar para que ande en vscode, solo se que si copias y pegas el comando en Git Bash arranca

 *  Executing task: "C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero" 

/usr/bin/bash: -c: line 1: unexpected EOF while looking for matching `"'

 *  The terminal process "C:\Program Files\Git\bin\bash.exe '-c', '"C:/Users/ivoja/Documents/pruebas-wollok/UNA CARPETA CON ESPACIOS/wollok-ts-cli-win-x64.exe" repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"'" terminated with exit code: 2. 
 *  Terminal will be reused by tasks, press any key to close it. 

Solo Argumentos Con Espacios

Con esta estructura de archivos

C:/Users/ivoja/Documents/pruebas-wollok
  mi proyecto wollokero
    package.json
    src
      pepita.wlk
  wollok-ts-cli-win-x64.exe

Reverti el ultimo commit para que al path de CLI no le ponga comillas

Git Bash ✔️

CMD ✔️

Powershell

Falla con las comillas dobles

 *  Executing task: C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero" 

🖥️  Initializing Wollok REPL for file  c:/Users/ivoja/Documents/pruebas-wollok/mi on c:/Users/ivoja/Documents/pruebas-wollok/mi
✗ File c:/Users/ivoja/Documents/pruebas-wollok/mi doesn't exist or is outside of project c:/Users/ivoja/Documents/pruebas-wollok/mi!

 *  The terminal process "C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -Command C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/ssss.wlk" --skipValidations --darkMode  -p "c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero"" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it. 

Le cambie en el codigo para que siempre use simples y arranco

 *  Executing task: C:/Users/ivoja/Documents/pruebas-wollok/wollok-ts-cli-win-x64.exe repl 'c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk' --skipValidations --darkMode  -p 'c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero' 

🖥️  Initializing Wollok REPL for file  c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero/src/pepita.wlk on c:/Users/ivoja/Documents/pruebas-wollok/mi proyecto wollokero

qué laburazo te mandaste!!

ahora me queda la duda. No se si invertir tanto tiempo en fixear la parte del ejecutable, sí obvio que los argumentos. Si querés lo hablamos el martes que viene así lo terminamos de definir.

@ivojawer
Copy link
Contributor

ahora me queda la duda. No se si invertir tanto tiempo en fixear la parte del ejecutable, sí obvio que los argumentos. Si querés lo hablamos el martes que viene así lo terminamos de definir.

Si, estaba pensando justo en eso cuando empece a ver los particularidades de cada shell. Dale, veamoslo el martes!

@ivojawer
Copy link
Contributor

ivojawer commented Sep 2, 2024

Sumo una cosita que encontre, TAL VEZ una luz al final del tunel

En esta linea que definimos la ejecucion por shell

new ShellExecution(shellCommand),

Podemos usar otro constructor que toma tanto el command como los arguments como ShellQuotedStrings
https://github.com/microsoft/vscode/blob/d4f566b68d21eb2ebbd2e3394b0bf0c829abae80/src/vs/workbench/api/common/extHostTypes.ts#L2259

Que segun la descripcion es "A string that will be quoted depending on the used shell."

@fdodino
Copy link
Contributor Author

fdodino commented Sep 2, 2024

Sumo una cosita que encontre, TAL VEZ una luz al final del tunel

En esta linea que definimos la ejecucion por shell

new ShellExecution(shellCommand),

Podemos usar otro constructor que toma tanto el command como los arguments como ShellQuotedStrings https://github.com/microsoft/vscode/blob/d4f566b68d21eb2ebbd2e3394b0bf0c829abae80/src/vs/workbench/api/common/extHostTypes.ts#L2259

Que segun la descripcion es "A string that will be quoted depending on the used shell."

Uh, qué groso!! Hay que darle una chance entonces

@ivojawer
Copy link
Contributor

ivojawer commented Sep 4, 2024

image

Efectivamente usando ShellQuotedString tiene en cuenta TODOS los casos raros que habia puesto arriba 🙌 Probe Windows y los casos mas normales de rutas con espacios en Mac.

Vole toda referencia a shells del proyecto

@fdodino
Copy link
Contributor Author

fdodino commented Sep 4, 2024

image image

Efectivamente usando ShellQuotedString tiene en cuenta TODOS los casos raros que habia puesto arriba 🙌 Probe Windows y los casos mas normales de rutas con espacios en Mac.

Vole toda referencia a shells del proyecto

image

Fuera de joda, impresionante tu hallazgo! No deja de parecerme increíble todo el laburo que hacés!

Copy link
Contributor

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

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

Maravilloso 🤩

Si te anda mandale merge - release - deploy - 🚀

client/src/test/commands.test.ts Outdated Show resolved Hide resolved
Comment on lines -46 to +47
delete actual[0]['canDecreaseHover']
delete actual[0]['canIncreaseHover']
delete actual[0]['canDecreaseVerbosity']
delete actual[0]['canIncreaseVerbosity']
Copy link
Contributor

Choose a reason for hiding this comment

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

???? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

No tengo ni idea por que pero al ejecutar el comando viene con estos atributos 🤷

    canDecreaseVerbosity: undefined,
    canIncreaseVerbosity: undefined,

Si me la tuviera que jugar diria que depende de la version de vscode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exacto, en versiones nuevas los wachiturros le cambiaron el nombre encima...

@PalumboN
Copy link
Contributor

PalumboN commented Sep 4, 2024

Fuera de joda, impresionante tu hallazgo! No deja de parecerme increíble todo el laburo que hacés!

Fer identificando la magia del Code Wizard Jawer 🧙 Jajajaja

+100

@ivojawer ivojawer merged commit 8e46d3a into master Sep 4, 2024
5 of 6 checks passed
@ivojawer ivojawer deleted the fix-#176-use-quotes-for-commands branch September 4, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants