lmorg / murex

A smarter shell and scripting environment with advanced features designed for usability, safety and productivity (eg smarter DevOps tooling)
https://murex.rocks
GNU General Public License v2.0
1.51k stars 27 forks source link

Tab completion with `export` causes panic #286

Closed itsjunetime closed 3 years ago

itsjunetime commented 3 years ago

If export is typed as the base command, typing a space + any number of characters and then hitting tab will cause a panic. The panic message is:

panic: runtime error: index out of range [0] with length 0

goroutine 617 [running]:
github.com/lmorg/murex/builtins/core/runtime.dumpExports(0xc00002a440, 0x9, 0x1518958)
    /Users/ian/go/src/github.com/lmorg/murex/builtins/core/runtime/runtime.go:249 +0x465
github.com/lmorg/murex/builtins/core/runtime.cmdRuntime(0xc00029a540, 0xc00009a870, 0xc00002a350)
    /Users/ian/go/src/github.com/lmorg/murex/builtins/core/runtime/runtime.go:136 +0x8ca
github.com/lmorg/murex/lang.executeProcess(0xc00029a540)
    /Users/ian/go/src/github.com/lmorg/murex/lang/process.go:261 +0x83f
created by github.com/lmorg/murex/lang.runModeNormal
    /Users/ian/go/src/github.com/lmorg/murex/lang/interpreter.go:177 +0x7e

This occurs regardless of how much you have typed after export. It may be related to #135, but I don't know for certain. If you think it's related, I can close this issue and move the information over to the other issue.

I'm seeing this on murex version 1.6.5100 BETA, macOS Big Sur 11.1. Let me know if there's more information you need from me.

lmorg commented 3 years ago

Hi @iandwelker

Looks like the bug is in the part that is supposed to be capturing the errors gracefully :facepalm: Fortunately it seems an easy fix and easy for me to add additional test cases around too. So I'll get on that shortly.

Are you're able to share your environmental variables prior to running export? If you're not already aware, you can do this by running env. Just make sure you obfuscate anything that is sensitive.

Lastly I think it's worth noting that you might have uncovered two bugs: the panic you posted above, but also export trying to complete with suggestions of variables that are already exported. So I'm glad you created this issue!

itsjunetime commented 3 years ago

Output of env (sorry it's a lot):

TERM_SESSION_ID=w0t0p0:2F0B334B-BD88-46D7-84D5-69315F1584A1
SSH_AUTH_SOCK=/private/tmp/com.apple.launchd.L8blqGjrJr/Listeners
LC_TERMINAL_VERSION=3.4.4
COLORFGBG=15;0
ITERM_PROFILE=Default
XPC_FLAGS=0x0
LANG=en_US.UTF-8
PWD=/Users/ian
SHELL=/usr/local/bin/murex
__CFBundleIdentifier=com.googlecode.iterm2
TERM_PROGRAM_VERSION=3.4.4
TERM_PROGRAM=iTerm.app
PATH=/Users/ian/.rvm/gems/ruby-3.0.0/bin:/Users/ian/.rvm/gems/ruby-3.0.0@global/bin:/Users/ian/.rvm/rubies/ruby-3.0.0/bin:/usr/local/bin:/usr/bin:/usr/local/sbin/:/Users/ian/theos/bin:/Users/ian/Documents/Coding/foss/flutter/bin:/usr/local/opt/curl/include:/Users/ian/.cargo/bin/:/Applications/Xcode12.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/:/usr/local/opt/sqlite/bin:/Users/ian/.rvm/bin/:/Users/ian/.rvm/bin/:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/TeX/texbin:/usr/local/share/dotnet:/opt/X11/bin:~/.dotnet/tools:/Library/Apple/usr/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/Applications/Wireshark.app/Contents/MacOS:/Users/ian/.rvm/bin
DISPLAY=/private/tmp/com.apple.launchd.aB3uXynbvI/org.macosforge.xquartz:0
LC_TERMINAL=iTerm2
COLORTERM=truecolor
COMMAND_MODE=unix2003
TERM=xterm-256color
HOME=/Users/ian
TMPDIR=/var/folders/wd/c4n4mj7s4ll0rjcwvwgf71qh0000gp/T/
USER=ian
XPC_SERVICE_NAME=0
LOGNAME=ian
ITERM_SESSION_ID=w0t0p0:2F0B334B-BD88-46D7-84D5-69315F1584A1
__CF_USER_TEXT_ENCODING=0x1F6:0x0:0x0
SHLVL=1
OLDPWD=/Users/ian
THEOS_DEVICE_IP=ipv4_address
THEOS_DEVICE_PASS=password
THEOS=/Users/ian/theos
ZSH=/Users/ian/.oh-my-zsh
ZSH_DISABLE_COMPFIX=true
EDITOR=nvim
HOMEBREW_NO_AUTO_UPDATE=1
DISABLE_AUTO_UPDATE=true
EXA_COLORS=di=32;3;0:ex=31;0;0:uu=35;0;0:gu=35;0;0
SPACESHIP_XCODE_SHOW_LOCAL=true
APPLE_ID=email@gmail.com
TIMEFMT=
================
CPU %P
user    %*U
system  %*S
total   %*E
ITC_VPN_USER=username
ITC_VPN_URL=vpn.server.com
NVM_DIR=/Users/ian/.nvm
PAGER=less
LESS=-R
LSCOLORS=Gxfxcxdxbxegedabagacad
STARSHIP_SHELL=zsh
STARSHIP_SESSION_KEY=3232214978179852
rvm_prefix=/Users/ian
rvm_path=/Users/ian/.rvm
rvm_bin_path=/Users/ian/.rvm/bin
rvm_version=1.29.12-next (master)
GEM_HOME=/Users/ian/.rvm/gems/ruby-3.0.0
GEM_PATH=/Users/ian/.rvm/gems/ruby-3.0.0:/Users/ian/.rvm/gems/ruby-3.0.0@global
MY_RUBY_HOME=/Users/ian/.rvm/rubies/ruby-3.0.0
IRBRC=/Users/ian/.rvm/rubies/ruby-3.0.0/.irbrc
RUBY_VERSION=ruby-3.0.0
_=/usr/local/bin/murex
AUTOJUMP_SOURCED=1
AUTOJUMP_ERROR_PATH=/Users/ian/Library/autojump/errors.log

This also led me to realize another thing: the error only appears when I am running murex inside zsh, which I had been doing when I ran into this issue. If I run murex as the startup shell, as opposed to inside of zsh, it doesn't panic at this point. It also doesn't panic inside bash or dash, just zsh, so I suspect it may be due to something exported inside my ~/.zshrc. I'll poke around a bit and see if I can find what exactly it is.

lmorg commented 3 years ago

Hi @iandwelker

Looks like it's the following env var that's triggering the bug:

TIMEFMT=
================
CPU %P
user    %*U
system  %*S
total   %*E

If truth be told, I've never seen a multi line environmental variable before.

Anyhow, I've completely rewritten how env vars are parsed (it was a lazy regex hack -- now I'm doing proper string parsing) and written a boat load of tests. I'm reasonably confident I've fixed the issue and since it's a pretty safe change I'll push it through to master for you now too.

I'll also kick off the automated build process to generate new binaries. The new version number is 1.7.0000 BETA

itsjunetime commented 3 years ago

Yeah, that's the only one I've ever seen as well. Thanks for getting that fixed up! And I do appreciate the lack of tab completion when typing in the name of the variable to export. However, this also disables directory tab completion when you're trying to export a directory as an environment variable. E.g. if you've typed export VAR=/Users/ian/Docume and hit tab, it will do nothing (instead of suggesting export VAR=/Users/ian/Documents). It this intended behavior?

Also, as a sidenote, would you be open to documentation PRs? I've noticed some areas that I think could be a bit better documented (no offense intended; this is a massive project so I wouldn't expect every tiny detail to be covered), and would love to file some PRs to improve documentation in those areas.

lmorg commented 3 years ago

It this intended behavior?

It was but that can be changed. I've raise an issue: https://github.com/lmorg/murex/issues/288

Also, as a sidenote, would you be open to documentation PRs?

Oh yes please!!! :D I'd very much welcome any contributions.

itsjunetime commented 3 years ago

It was but that can be changed. I've raise an issue: #288

My personal preference would be to have autocompletion when a variable's value is being set to a directory. However, that's just a personal preference, and could cause issues if someone's actually trying to insert a tab into a variable value, so (unless you have a strong opinion on the matter) the best option may be to just make it user-configurable?

Oh yes please!!! :D I'd very much welcome any contributions.

Cool cool, I've forked the repo and I hope to submit a PR/some PRs soon :)

lmorg commented 3 years ago

My personal preference would be to have autocompletion when a variable's value is being set to a directory. However, that's just a personal preference, and could cause issues if someone's actually trying to insert a tab into a variable value, so (unless you have a strong opinion on the matter) the best option may be to just make it user-configurable?

It's very easy to add support for directories in addition to other fields. That code has already been written. What hasn't been written is support to load new tab completion rules upon a =. But I've planned to implement that feature of a while now.

As for tabs, that could just be escaped: \t. eg:

~ » set tab=\t
~ » out .$tab.
.       .

Cool cool, I've forked the repo and I hope to submit a PR/some PRs soon :)

Awesome, thank you. I should warn you that documentation is compiled in murex from *_doc.yaml files (eg the below). The rational being that the compiler works a little like a CMS and can link related pages as well as common header/footers/etc. Really I should write some documentation on how the documentation is compiled lol. But worst case scenario I can take your contributions and copy/paste back into the YAML

- DocumentID: global
  Title: >+
    `global`
  CategoryID: commands
  Summary: >-
    Define a global variable and set it's value
  Description: |-
    Defines, updates or deallocates a global variable.
  Usage: |-
# Assume data type and value from STDIN
<stdin> -> global var_name

# Assume value from STDIN, define the data type manually
<stdin> -> global datatype var_name

# Define value manually (data type defaults to string; `str`)
global var_name=data

# Define value and data type manually
global datatype var_name=data

# Define a variable but don't set any value
global var_name
global datatype var_name
```

Examples: |- As a method:

```
» out "Hello, world!" -> global hw
» out "$hw"
Hello, World!
```

As a function:

```
» global hw="Hello, world!"
» out "$hw"
Hello, World!
```

Flags: Detail: |-

Deallocation

You can unset variable names with the bang prefix:

```
!global var_name
```

{{ include "gen/includes/variables.inc.md" }}

Synonyms: