Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

Ignore input lines starting with # (comments) #13

Merged
merged 2 commits into from
Mar 19, 2019

Conversation

walfie
Copy link
Contributor

@walfie walfie commented Mar 18, 2019

Readline (and thus rlwrap) has the ability to comment out the current
input line by pressing esc followed by #. However, kubectl-repl will
execute this as kubectl #insert comment here which shows the help
text. This updates the repl to ignore lines beginning with #.

Readline (and thus rlwrap) has the ability to comment out the current
input line by pressing `esc` followed by `#`. However, kubectl-repl will
execute this as `kubectl #insert comment here` which shows the help
text. This updates the repl to ignore lines beginning with `#`.
@Mikulas
Copy link
Member

Mikulas commented Mar 19, 2019

Interesting. Currently this almost works as it leaves the interpretation on the underlying shell such as # prod test ; # echo foo (which is hardcoded to bash #2). But that is explicitly handled with the ; prefix.

We might consider adding the # to the shell builtin instead. https://github.com/Mikulas/kubectl-repl/blob/master/main/builtin-shell.go#L14

Also refactor whitespace trimming (which includes newlines)
@Mikulas
Copy link
Member

Mikulas commented Mar 19, 2019

Awesome, thank you very much :)

@walfie
Copy link
Contributor Author

walfie commented Mar 19, 2019

That makes sense, I was wondering about that as well. I've pushed a commit that moves it to the builtin shell.

Also included a minor whitespace handling change -- I noticed TrimSpace also trims newline characters, and there was an existing call to trim newlines above, so I just combined them.

@Mikulas Mikulas merged commit a55cff9 into clusterise:master Mar 19, 2019
@walfie walfie deleted the ignore-comment branch March 19, 2019 14:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants