jbrooksuk / Sublime-Evaluate

Selection evaluation in Sublime Text
https://james-brooks.uk
25 stars 6 forks source link

! echo hello evaluates to hello<0x0d> #9

Closed evandrocoan closed 6 years ago

evandrocoan commented 6 years ago

back2

Environment

weichuliu commented 6 years ago

Thanks for reporting.

What happened is that under windows, shellscript returns "CRLF" as the line ending. On the other hand, the evaluation only trimmed the last "LF".

I'll make a PR for this.

weichuliu commented 6 years ago

@evandrocoan Since I don't have a windows to test, can you also show the result about ! seq 5? Need to know if need to fix every line ending or just the last one I trimmed in a wrong way.

evandrocoan commented 6 years ago

The result was this:

image

weichuliu commented 6 years ago

There's no CR even at the last line? Did you change any settings between two screenshot?

Can you evaluate the following 3 respectively?

! seq 5

! printf '\r\n\r\n'

! echo hello
evandrocoan commented 6 years ago

back

When I try to evaluate all at once, only the first line is evaluated.

weichuliu commented 6 years ago

I have no idea which shell you are using on Windows. In bash ! is interpreted as "not" but still you can get the output.

On my machine I got image

Except for that, the CR issue can be addressed. I'm making PR for it

evandrocoan commented 6 years ago

My shell is this:

$ sh --version
GNU bash, version 4.4.12(3)-release (x86_64-unknown-cygwin)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

$ seq --version
seq (GNU coreutils) 8.26
Packaged by Cygwin (8.26-2)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Ulrich Drepper.

Running these commands directly on the shell does this:

image

weichuliu commented 6 years ago

I have no idea why the rest 2 lines are not evaluated. How about running this from your shell:

sh -c "seq 5
! printf '\r\n\r\n'
! echo hello"
evandrocoan commented 6 years ago

Runs fine:

image

weichuliu commented 6 years ago

Ok, one possibility that I can think of is when Python runs your code on windows, the input is using '\n' instead of '\r\n', so your windows shell get choked.

If that is the case, the PR will also fix your second problem.

It'd be very appreciate if you can test my branch before it being merged.

(How-to)

  1. Find the package location using "Preferences ▶ Browse Packages" (http://www.sublimetext.com/docs/3/packages.html), you should be able to find a "Packages" dir and a "Installed Packages" dir.

  2. cd to "Packages" dir, then do the following:

/path/to/Packages $ git clone https://github.com/weichuliu/Sublime-Evaluate.git
/path/to/Packages $ git checkout issue-9-shell-evaluate-handles-crlf
  1. Test the cases above from Sublime UI

  2. Remove the cloned dir so it won't override the installed package forever

evandrocoan commented 6 years ago

back

weichuliu commented 6 years ago

Another guess is that Python is actually calling cmd or powershell instead of bash, this can be confirm by checking if your echo is bash echo or cmd echo

Can you do the following from Sublime?

! echo
evandrocoan commented 6 years ago

image

weichuliu commented 6 years ago

So, you are calling CMD.

In such case you cannot evaluate all three lines at the same time, since ! has different usage in CMD syntax. That is expected.

That also explains your \r\n only when echo, because you are running windows echo.

So, since windows doesn't provide a posix shell, that's all I can do.

(Actually you might already found out how it is a pain-in-the-ass to shell-out from windows. Can I just suggest you to switch to Linux or mac? 😆

weichuliu commented 6 years ago

If you want to use a posix shell from Sublime-Evaluate anyway, I will suggest you to google if Python can call a certain shell when Popen. Then you can modify the evaluate.py to have it pointing to your shell's path.

However, since there is not a unify "shell binary location" under windows, I'm afraid there's not a way to make this repo supporting such feature.

weichuliu commented 6 years ago

@evandrocoan Can you test the latest commit with

! echo 😊

To see if unicode char breaks the evaluation?

evandrocoan commented 6 years ago

back

On shell it works: image

As I am using the Windows' echo command, not patches should be done trying to fix echo for it.

The only thing that could be done is to not run this package if it is trying to use Window tools. This could be done detecting whether echo is from windows or linux. In case it is from Windows, then find where sh.exe is located and add its directory path to this package popen.command before trying to run the shell commands. This path detection should be done only one time on plugin initialization.

weichuliu commented 6 years ago

General speaking, fixing \r\n to \n on windows sounds like a desired behavior for me.

On the other hand, you can never expect *sh.exe is on a certain windows machine, not to mention finding it somewhere from the machine. This violates the principle of least astonishment. So I think plugin should open cmd.

evandrocoan commented 6 years ago

Trying to make this package patched for individual Windows components seems worse as there are much more things to fix other than just the echo command.

If you do not use Windows, please, do not try to fix it as you do not know the daily usage/experience for a Windows user. Just let some Windows dev get interested on it.

On the other hand, you can never expect *sh.exe is on a certain windows machine

This package should me marked as Linux only. However, beyond that, you can run it on a Windows machine, if you have Linux tools installed, as Cygwin POSIX environment. And this is my suggestion here. There is no problem in searching for linux tools on the system path, as they should be present on a Windows machine with Linux tools installed. But if you do not like it, could just add a settting for the user to set their /bin folder with Linux tools for Windows.

weichuliu commented 6 years ago
  1. The PR not only fix MS echo, but any \r\n newline char. Since by default Windows use \r\n as EOL, generally speaking the plugin should handle it.

  2. The fact is that Windows has default shell called cmd. If you agree with that, current plugin behavior, to shell out to cmd, sounds reasonable. Since the plugin is working fine on all platforms, I don't know why it should be marked as Linux only.

  3. I don't have bandwidth to make this plugin configurable (e.g. set *sh.exe so plugin uses it instead of default shell). Since you are a Windows dev, why not make a PR for your requirement?