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

make prodigy command respect display-buffer-alist #149

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

Conversation

lem102
Copy link

@lem102 lem102 commented Nov 28, 2024

closes #146

Copy link
Collaborator

@DamienCassou DamienCassou left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Can you please check the tests? They don't pass on the CI.

@lem102 lem102 force-pushed the prodigy-command-respect-display-buffer-alist branch from 29e2993 to 63d76e5 Compare December 3, 2024 20:06
@lem102
Copy link
Author

lem102 commented Dec 3, 2024

I'm struggling to run the tests locally. Are the docs for running the tests up to date? Running make doesn't seem to do anything related to running the tests.

@DamienCassou
Copy link
Collaborator

I'm struggling to run the tests locally.

you can always refer to how GitHub actions are implemented to learn how this works. This is in .github/workflows/test.yml:

$ make ci-dependencies
$ nix shell \
  --extra-experimental-features flakes \
  --extra-experimental-features nix-command \
  nixpkgs#nodePackages.coffee-script \
  --command make check

The Makefile is implemented with makel. Reading its documentation may help understand how things work.

The make ci-dependencies downloads dependencies (look at the first line of the Makefile) if they are not checked out in the parent folder already.

Then, the nix shell command downloads coffee-script before executing make check. The test that fails is in features/quit.feature and this doesn't need coffee-script. As a result, you can replace the whole nix shell command with just:

$ make test-ecukes

@DamienCassou
Copy link
Collaborator

I think the overall issue with this PR is that the quit.feature test suite requires prodigy-mode to only be executed once (to keep the service marks when reopening prodigy) whereas the PR executes prodigy-mode each time the user re-opens the prodigy buffer.

The PR can be fixed with:

diff --git a/prodigy.el b/prodigy.el
index e9bb211..07d1a57 100644
--- a/prodigy.el
+++ b/prodigy.el
@@ -1550,9 +1550,10 @@ (define-derived-mode prodigy-view-mode special-mode "Prodigy-view"
 (defun prodigy ()
   "Manage external services from within Emacs."
   (interactive)
-  (let ((buffer (get-buffer-create prodigy-buffer-name)))
+  (let ((buffer-p (prodigy-buffer))
+        (buffer (get-buffer-create prodigy-buffer-name)))
     (with-current-buffer buffer
-      (prodigy-mode)
+      (when (not buffer-p) (prodigy-mode))
       (prodigy-start-status-check-timer))
     (pop-to-buffer buffer)))
 
-- 
2.47.0

What still needs to be done is to add a new test scenario covering the new behavior.

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.

cannot use major mode in display-buffer-alist to control prodigy-mode buffer placement
2 participants