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

change mini-window height during phpactor-status #124

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

kermorgant
Copy link
Contributor

I'm not really happy to change such a global variable but I did not find a working solution to have it changed only temporarily.

fixes #123

@kermorgant kermorgant requested a review from zonuexe July 2, 2019 15:04
phpactor.el Outdated
@@ -659,6 +659,7 @@ function."
(defun phpactor-status ()
"Execute Phpactor RPC status command, and pop to buffer."
(interactive)
(setq max-mini-window-height 18)
Copy link
Member

Choose a reason for hiding this comment

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

Can it be enclosed by let?

(let ((max-mini-window-height 18))
  (apply #'phpactor-action-dispatch (phpactor--rpc "status" [])))

let temporarily overwrites variables and restores them.

Copy link
Member

Choose a reason for hiding this comment

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

Can you make the literal value 18 a custom variable instead of a magic number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zonuexe done (with a note in the README about it)

@kermorgant
Copy link
Contributor Author

kermorgant commented Jul 3, 2019

Unfortunately, I had tried but let doesn't help in that situation.

I also tried temporarily taking a backup of original value and restoring it in a statement at the end but that last part (restoration) did not work.

@kermorgant
Copy link
Contributor Author

Hi @zonuexe

Admitedly, this change alters a variable outside phpactor's own scope, which is not ideal.

However, for user trying this package for the first time, having all the information from phpactor-status visible can be critical (specially the working-directory).

wdyt ?

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.

ensure complete output of phpactor-status is readable
2 participants