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

added chef-shell prompt to inf-ruby-prompt-format #108

Merged
merged 2 commits into from Aug 15, 2017
Merged

added chef-shell prompt to inf-ruby-prompt-format #108

merged 2 commits into from Aug 15, 2017

Conversation

roymath-zz
Copy link
Contributor

Also changed to defvar instead of defconst, for easier customization.

inf-ruby.el Outdated
(concat
(mapconcat
#'identity
'("\\(^%s> *\\)" ; Simple
"\\(^chef ([\\.[:digit:]]+)> *\\)" ; Chef Shell: e.g. "chef (13.2.20)> "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the Chef console prompt look in two cases:

  • Right after start
  • When you input 2 + and press RET

?

@roymath-zz
Copy link
Contributor Author

roymath-zz commented Jul 20, 2017 via email

@dgutov
Copy link
Collaborator

dgutov commented Jul 20, 2017

And when you finish the expression by typing 3 and RET, it goes back to chef (13.2.20)>?

@roymath-zz
Copy link
Contributor Author

Correct.

Ohai2u @chef-node!
chef (13.2.20)> 2 + 
chef >            3
 => 5
chef (13.2.20)> 1+2
 => 3
chef (13.2.20)> 

@dgutov
Copy link
Collaborator

dgutov commented Jul 20, 2017

In that case, its scheme doesn't fit inf-ruby's assumptions about the prompt's behavior.

You should probably change inf-ruby-first-prompt-pattern and inf-ruby-prompt-pattern buffer-locally instead, and they are already defvar-s. Doing that in inf-ruby-mode-hook (with a check that the current buffer runs Chef console, somehow) seems appropriate.

@dgutov
Copy link
Collaborator

dgutov commented Jul 20, 2017

inf-ruby-prompt-pattern has to match any prompt, and inf-ruby-first-prompt-pattern has to match only the non-continuation ones.

@roymath-zz
Copy link
Contributor Author

Understood.. this snippet seems to do the trick

(add-hook 'inf-ruby-mode-hook
          '(lambda() (let ((p "\\(^chef \\(([\\.[:digit:]]+)>\\)? *\\)"))
		       (setq inf-ruby-first-prompt-pattern p)
		       (setq inf-ruby-prompt-pattern p))))

@dgutov
Copy link
Collaborator

dgutov commented Jul 20, 2017

If both regexps are the same, you won't get protection against completion messing up with the input in the continuation lines, and the "get old input" commands won't be able to fetch multiline inputs (the ones interspersed with continuation prompts).

@dgutov
Copy link
Collaborator

dgutov commented Jul 20, 2017

Also: you never need to quote a lambda form.

@roymath-zz
Copy link
Contributor Author

roymath-zz commented Jul 21, 2017

Thanks for the tips. I tried the following:

  • keep a single RE
  • concat with previous patterns, so that first-prompt is unique
(add-hook 'inf-ruby-mode-hook
          (lambda() (let ((p "\\|\\(^chef \\(([\\.[:digit:]]+)>\\)? *\\)"))
                       (setq inf-ruby-first-prompt-pattern (concat inf-ruby-first-prompt-pattern p))
                       (setq inf-ruby-prompt-pattern (concat inf-ruby-prompt-pattern p)))))

; These tests work
;(string-match inf-ruby-first-prompt-pattern "chef (13.2.20)> ")                                                                             
;(string-match inf-ruby-prompt-pattern "chef (13.2.20)> ")                                                                                   
;(string-match inf-ruby-prompt-pattern "chef >     ")                                                                                        

However, not sure if I got it right, since I don't get autocomplete working at the second level prompt..

chef (13.2.20)> node. |<- autocomplete works here

Ohai2u @chef-node!
chef (13.2.20)> (
chef >              node. |<- autocomplete DOESN'T work here

@dgutov
Copy link
Collaborator

dgutov commented Jul 22, 2017

I don't get autocomplete working at the second level prompt

That's intentional and a good thing: code completion is implemented by evaluating strings in the same process, and if the REPL has already been sent an incomplete expression, completion will both fail to work and mess up said expression. That's what I meant by "messing up with the input" in a comment above.

@roymath-zz
Copy link
Contributor Author

Ok.. so I guess I will close this PR. If you like, I can add a note to the documentation about this approach for those that might be wondering the same as I did.

@dgutov
Copy link
Collaborator

dgutov commented Jul 23, 2017

I can add a note to the documentation about this approach for those that might be wondering the same as I did

Yeah, that sounds like a good idea.

@roymath-zz
Copy link
Contributor Author

I've added a comment blurb, and removed the defvar commit (using a rebase that hopefully didn't screw anything up).

@dgutov
Copy link
Collaborator

dgutov commented Jul 28, 2017

Thanks. I think I'd rather have it in the wiki, though. Along with an explanation why the two prompts regexps should not be the same.

The README would have a link to it, under a heading Custom Prompts. Sound good?

@roymath-zz
Copy link
Contributor Author

Sure.. however I don't see anything yet in the wiki: https://github.com/nonsequitur/inf-ruby/wiki

Let me know if there is anything I can help with, or whether you'd rather add it. Fine either way - and thanks for the helpful comments. My elisp is better from having gotten into this discussion.

@dgutov
Copy link
Collaborator

dgutov commented Jul 28, 2017

Let me know if there is anything I can help with

Yes, please go ahead. I can edit the result after.

And no problem!

@roymath-zz
Copy link
Contributor Author

I've added to the wiki and edited the README.

@dgutov
Copy link
Collaborator

dgutov commented Aug 2, 2017

I don't see the suggested heading in the README. Nor this:

Along with an explanation why the two prompts regexps should not be the same.

I'd also prefer a dedicated wiki page rather that putting in at Home.

@roymath-zz
Copy link
Contributor Author

Made some edits.. please review and edit if needed

@dgutov
Copy link
Collaborator

dgutov commented Aug 2, 2017

Looks better, thanks. Now waiting for the requested change in the README addition (also, please drop one of the empty lines).

@roymath-zz
Copy link
Contributor Author

Not sure what blank line you're referring to. Anyhow, the heading change is in.

@dgutov
Copy link
Collaborator

dgutov commented Aug 15, 2017

Not sure what blank line you're referring to.

The second of the two empty lines at the end of your diff hunk. But never mind that now.

Thanks for the wiki page.

@dgutov dgutov merged commit 4893dd6 into nonsequitur:master Aug 15, 2017
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 this pull request may close these issues.

None yet

2 participants