gnebbia / kb

A minimalist command line knowledge base manager
GNU General Public License v3.0
3.17k stars 104 forks source link

Edit in external editor is broken #19

Closed graphixillusion closed 4 years ago

graphixillusion commented 4 years ago

Expected Behavior

Edit artifact in external editor (specified in the %EDITOR% env variable)

Actual Behavior

Traceback (most recent call last):
  File "__main__.py", line 20, in <module>
  File "main.py", line 62, in main
  File "main.py", line 52, in dispatch
  File "commands\edit.py", line 44, in edit
  File "commands\edit.py", line 73, in edit_by_id
  File "subprocess.py", line 304, in call
  File "subprocess.py", line 756, in __init__
  File "subprocess.py", line 1100, in _execute_child
  File "subprocess.py", line 511, in list2cmdline
TypeError: argument of type 'WindowsPath' is not iterable
[7080] Failed to execute script __main__

Steps to Reproduce the Problem

  1. Add a file as artifact
  2. Edit the file with this command: kb edit --id "number"

Specifications

gnebbia commented 4 years ago

Yes Sir, Thanks for reporting the issue, unluckily kb is still in alpha for windows! Anyway we are currently working on fully supporting Windows with kb 0.1.3, if you wait just some days, I think in the weekend it will be ready. Let's keep the issue open until then ok? Thanks again

gnebbia commented 4 years ago

Ok, I just checked that out, it actually is not a bug, but you should put your EDITOR environment variable between double quotes. So instead of having: EDITOR=C:\Program Files\blabla\my editor.exe You MUST have: EDITOR="C:\Program Files\blabla\my editor.exe"

I just tried in a virtual machine and it works! Thanks for pointing me out, so that I can include this within the docs! Can you let me know if that fixed it for you?

gnebbia commented 4 years ago

Can you also please let me know if it is clear from the documentation how to set the EDITOR environment variable? Let me know if it is clear or if I should add some details. To conclude, I think the current release v0.1.2 is already ok to use with Windows.

graphixillusion commented 4 years ago

@gnebbia I've tried what you just suggested but i'm still getting the errors. Actually the quotes are needed if there are some spaces in the path, but in my case i don't have any space. Btw i have inserted the quotes but i still getting errrors.

echo %EDITOR% "D:\notepadplusplus\notepad++.exe"

kb edit --id 0

Traceback (most recent call last):
  File "__main__.py", line 20, in <module>
  File "main.py", line 62, in main
  File "main.py", line 52, in dispatch
  File "commands\edit.py", line 44, in edit
  File "commands\edit.py", line 73, in edit_by_id
  File "subprocess.py", line 304, in call
  File "subprocess.py", line 756, in __init__
  File "subprocess.py", line 1100, in _execute_child
  File "subprocess.py", line 511, in list2cmdline
TypeError: argument of type 'WindowsPath' is not iterable
[5088] Failed to execute script __main__

I got the same error even if i completely delete the EDITOR var from the system.

The strange thing is that if i try to compile it from source, it works ok without quotes in the editor's path: if i insert the quotes i get this error

Traceback (most recent call last):
  File "C:\Users\User\scoop\apps\python\current\Scripts\kb-script.py", line 11, in <module>
    load_entry_point('kb-manager==0.1.2', 'console_scripts', 'kb')()
  File "C:\Users\User\scoop\apps\python\current\lib\site-packages\kb-0.1.1-py3.8.egg\kb\main.py", line 62, in main
    dispatch(cmd, cmd_params, config=DEFAULT_CONFIG)
  File "C:\Users\User\scoop\apps\python\current\lib\site-packages\kb-0.1.1-py3.8.egg\kb\main.py", line 52, in dispatch
    return COMMANDS[function](*args, **kwargs)
  File "C:\Users\User\scoop\apps\python\current\lib\site-packages\kb-0.1.1-py3.8.egg\kb\commands\edit.py", line 49, in edit
    call([config["EDITOR"], Path(category_path, artifact.title)])
  File "C:\Users\User\scoop\apps\python\current\lib\subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "C:\Users\User\scoop\apps\python\current\lib\subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\User\scoop\apps\python\current\lib\subprocess.py", line 1307, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2]
yihong0618 commented 4 years ago

@gnebbia @graphixillusion

Its also a bug of pathlib see https://bugs.python.org/issue33617

I think I can take a pr later.


And I think for windows user can we suggest that use vscode install of vim or nodepad++. In windows cmd, I think code abc is easy to use.

yihong0618 commented 4 years ago

@graphixillusion @gnebbia

Fixed, please check. And fix ci.

gnebbia commented 4 years ago

Ok, I really agree on the CI tests, I think I will pull those, but for the Windows bug, I am trying to reproduce it, and still can't. I tried to uninstall all kb versions and then I did: pip install -U kb-manager

At this point it works flawlessly... I tried kb list kb edit --id 0

Are we sure this is not a problem related to the python version? Also, notice that using shell=True may be insecure due to command injection attacks. Let's work toward understanding this issue and solving it.

What version of python are you using?

gnebbia commented 4 years ago

for what concerns "And I think for windows user can we suggest that use vscode install of vim or nodepad++. In windows cmd, I think code abc is easy to use." I agree with that, although I don't know what code abc is.

yihong0618 commented 4 years ago

Use python3.6 in windows cmd。

yihong0618 commented 4 years ago

Vscode shortcut in terminal. “code abc.py” will open a file in vscode. like vim abc.py

yihong0618 commented 4 years ago

Can you use python3.6 maybe 3.7 in cmd using kb add —title abc

gnebbia commented 4 years ago

Ok I am working with it!

gnebbia commented 4 years ago

Ok I tried with python 3.6 on Windows, and noticed that we can apply all of your changes without the shell=True. Can you confirm that? It seems to be enough to convert Path(s) to strings.

yihong0618 commented 4 years ago

Let me have a try. Can we use VsCode code as default editor for windows?

gnebbia commented 4 years ago

yihong I didn't test how it works for VsCode code, if you say that it works flawlessly yes why not.