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

Fresh install not working on "git for windows" terminal #14

Closed
MartyLake opened this issue Jun 20, 2019 · 7 comments · Fixed by #15
Closed

Fresh install not working on "git for windows" terminal #14

MartyLake opened this issue Jun 20, 2019 · 7 comments · Fixed by #15

Comments

@MartyLake
Copy link
Contributor

MartyLake commented Jun 20, 2019

Hey,
I tried following the tutorial from a "git for windows" terminal.
Cloning, sourcing from .bashrc file, creating a .ok file exactly like the example, but ok l and ok 1 show no output.
Do you have any idea how to debug this further ?

$ cat .ok
# My first ok-command
echo "Hi $USER, the time when pressed enter was $(date "+%H:%M:%S")"

$ ok

$ ok 1

$ ok l

$ ok -v
comment_align: 1
terminal_width: 271

$ ok -h
Usage: ok [options] <number> [script-arguments..]
       ok command [options]

command (use one):
  <number>            Run the <number>th command from the '.ok' file.
  l, list             Show the list from the '.ok' file. Default command.
  L, list-once        Same as list, but only show when pwd is different from when the list was last shown.
  p, list-prompt      Show the list and wait for input at the ok-prompt (like --list and <number> in one command).
  h, help             Show this usage page.
options:
  -c, --comment_align N  Level of comment alignment. See $_OK_COMMENT_ALIGN
  -v, --verbose       Show more output, mostly errors. Also it shows environment-variables in this screen.
  -q, --quiet         Only show really necessary output, so surpress echoing the command.
  -f, --file <file>   Use a custom file instead of '.ok'; use '-' for stdin
  -a, --alias <name>  When using 'ok' in an alias, <name> is used to keep the history correct when used with 'list-prompt'.
  -V, --version       Show version number and exit
  -h, --help          Show this help screen
script-arguments:
  ...                 These are passed through, when a line is executed (you can enter these too at the ok-prompt)

$ bash --version
GNU bash, version 4.4.23(1)-release (x86_64-pc-msys)
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.

$ python --version
Python 2.7.16

@MartyLake MartyLake changed the title Not working on windows Fresh install not working on "git for windows" terminal Jun 20, 2019
@secretGeek
Copy link
Owner

Intriguing! I'm on WSL, bash version 4.4.19.

What do you get for:

$ python3 --version
Python 3.6.5

I didn't have python 2, (i.e. python) -- so installed it now.

This results in:

$ python --version

Python 2.7.15rc1

Still can't repro the issue, though.

@MartyLake
Copy link
Contributor Author

$ python3 --version
Python 3.7.3
$ python --version
Python 2.7.16

:)

@MartyLake
Copy link
Contributor Author

So I bisected through the python file and commented out the section about utf8 preparation and it seems like it is not silently crashing anymore :)

diff --git a/ok-show.py b/ok-show.py
index cd9ea8e..dbbe09e 100755
--- a/ok-show.py
+++ b/ok-show.py
@@ -144,12 +144,12 @@ def main():
         print('terminal_width:', args.terminal_width)
 
     #setup UTF-8
-    if sys.stdin.encoding != 'UTF-8':
-        sys.stdin = codecs.getreader('utf8')(sys.stdin, 'strict')
-    if sys.stdout.encoding != 'UTF-8':
-        sys.stdout = codecs.getwriter('utf-8')(sys.stdout, 'strict')
-    if sys.stderr.encoding != 'UTF-8':
-        sys.stderr = codecs.getwriter('utf-8')(sys.stderr, 'strict')
+    # if sys.stdin.encoding != 'UTF-8':
+    #     sys.stdin = codecs.getreader('utf8')(sys.stdin, 'strict')
+    # if sys.stdout.encoding != 'UTF-8':
+    #     sys.stdout = codecs.getwriter('utf-8')(sys.stdout, 'strict')
+    # if sys.stderr.encoding != 'UTF-8':
+    #     sys.stderr = codecs.getwriter('utf-8')(sys.stderr, 'strict')
     # prepare
     lines = sys.stdin.readlines()
     p_lines = parse_lines(lines)

@secretGeek
Copy link
Owner

Awesome research Marty. Absolute top notch work.

Previously, that code said just:

UTF8Reader = codecs.getreader('utf8')	
    sys.stdin = UTF8Reader(sys.stdin)

and was fixed with comment "Fixed an Unicode bug (ok 9 works again)"

Maybe @doekman has some insight to offer. Maybe we could write a little test python file you can run in your environment so we work out something that works for all environments.

@doekman
Copy link
Collaborator

doekman commented Jul 9, 2019

There are two problems. I recently made python3 be preferred above python2. This was fine on my machine (Python 3.7.3 on macOS Mojave) but not on others (Debian Stretch is on 3.5.3) which I discovered just yesterday...

Using Python 3.5 surfaces a bug in the "Fixed an Unicode bug"-fix mentioned by secretGeek. Weird it's also happening on Python 3.7.3 on Windows...

I think a workaround is force using python 2:

export _OK__PATH_TO_PYTHON=$(command -v python)

Can you confirm this, @MartyLake ?

I will have to investigate why I put the part below in, because clearly it's not working...

#setup UTF-8
if sys.stdin.encoding != 'UTF-8':
    sys.stdin = codecs.getreader('utf8')(sys.stdin, 'strict')
if sys.stdout.encoding != 'UTF-8':
    sys.stdout = codecs.getwriter('utf-8')(sys.stdout, 'strict')
if sys.stderr.encoding != 'UTF-8':
    sys.stderr = codecs.getwriter('utf-8')(sys.stderr, 'strict')

@MartyLake
Copy link
Contributor Author

@doekman hey, I can confirm that the workaround of forcing python2 works for me :)

@doekman doekman mentioned this issue Jul 24, 2019
@doekman
Copy link
Collaborator

doekman commented Aug 17, 2019

This now works with python3 too.
You can unset _OK__PATH_TO_PYTHON.

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 a pull request may close this issue.

3 participants