Switch to use transient.el by pkryger · Pull Request #26 · wbolster/emacs-python-pytest · GitHub

pkryger · 2020-08-02T16:18:41Z

transient is a replacement to magit-popup and even magit has already switched to use transient.

This fixes #18.

The switch is pretty idiomatic, however there are a few differences.

The default layout is slightly different but I didn't try to replace it as the new one seems to match what magit is presenting these days. I think that the new layout resembles more what magit is presenting these days, but I don't have any scientific method for that. Happy to work on changing to original layout if this is desired.

The concept of :default-action seems to be gone, so I did not replace it.

The magit-with-pre-popup-buffer doesn't seem to have an obvious replacement, however the documentation for transient--original-buffer suggest that the transient popup is not changing the current buffer. I validated that with a check that the current-buffer returns expected pytest buffer while invoking python-pytest via python-pytest-popup.

wbolster

thanks for your contribution! this is looking quite good already. i left a few comments; nothing major.

pkryger · 2020-08-03T13:56:39Z

Thank you for review. Please let me know which of fixes to comments you'd like to see in before merging.

wbolster · 2020-08-03T18:58:02Z

i responded inline to most comments. happy to merge this after making a tiny adjustment.

i will experiment with shortening some names later. i'll ping you for feedback once i have a pr for it, if you don't mind.

i actually started a branch locally to switch to transient.el but stopped working on it two minutes later since ~~i got distracted~~ life happened.

and then all of a sudden someone else did all the work. 👍😄

wbolster · 2020-08-03T19:00:35Z

oh one more thing: defining python-pytest-arguments in init.el was a good way to set defaults before.

is there a clear replacement for it? if so, it may be worth adding to the readme.

wbolster · 2020-08-03T19:04:22Z

oh yet another thing, maybe use python-pytest-dispatch as the name of the function (and helpers). it seems to be the convention?

i guess an alias for the obsolete name (defsubst iirc, typing from phone) would help with upgrades.

but it's not required and i can do it later.

`transient` is a replacement to `magit-popup` and even `magit` has already switched to use `transient`. The switch is pretty idiomatic, however there are a few differences. The default layout is slightly different but I didn't try to replace it as the new one seems to match what `magit` is presenting these days. The concept of `:default-action` seems to be gone, so it is not replaced. The `magit-with-pre-popup-buffer` doesn't seem to have an obvious replacement, however the documentation for `transient--original-buffer` suggest that the `transient` is not changing the current buffer.

pkryger · 2020-08-03T20:17:04Z

i'll ping you for feedback once i have a pr for it, if you don't mind.

Sure. Happy to help with it.

oh yet another thing, maybe use python-pytest-dispatch as the name of the function (and helpers). it seems to be the convention?

Let me see what I can do today.

is there a clear replacement for it? if so, it may be worth adding to the readme.

Perhaps there's some programmatic way, but I'm not aware of it. What I know is transient has a concept of saving your transient state as a default. Just C-x C-s after you selected all options and this will be the new default (persistent) or C-x s and it will persist in until emacs restart. I will update readme.

wbolster · 2020-08-04T13:00:58Z

nice, i see you did the rename to -dispatch, thanks!

pkryger

Will update with the removal of fmakeunbound/makeunbound

With transient, an `eval-buffer` is picking up new definition of `python-pytest-dispatch` (the popup).

wbolster · 2020-08-04T18:45:47Z

merged, thanks a lot for doing all the hard/boring (choose yourself) work!

pkryger · 2020-08-04T18:49:03Z

Pleasure. It was really nice to work with you!

Make the ‘-v’ option cycle between nothing, ‘--verbose’, and ‘--verbose --verbose’. This was suggested before (#22 (review)) but wasn't possible at the time of #22 because it seemed impossible due to magit-popup.el limitations. Now that the dispatch menu uses transient.el instead of magit-popup.el (#18, #26), it can be implemented after all

Make the ‘-v’ option cycle between nothing, ‘--verbose’, and ‘--verbose --verbose’. This was suggested before (#22 (review)) but wasn't possible at the time of #22 because it seemed impossible due to magit-popup.el limitations. Now that the dispatch menu uses transient.el instead of magit-popup.el (#18, #26), it can be implemented after all.

Make the ‘-v’ option cycle between nothing, ‘--verbose’, and ‘--verbose --verbose’. This was suggested before (#22 (review)) but wasn't possible at the time of #22 due to magit-popup.el limitations. Now that the dispatch menu uses transient.el instead of magit-popup.el (#18, #26), it can be implemented after all.