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

Redesign dispatch menu #28

Merged
merged 4 commits into from
Aug 10, 2020
Merged

Redesign dispatch menu #28

merged 4 commits into from
Aug 10, 2020

Conversation

wbolster
Copy link
Owner

@wbolster wbolster commented Aug 4, 2020

  • use better groupings
  • use multi-column visual layout
  • add some more flags
  • make all flags start with either - or -- (mostly mimicking pytest flags)

@wbolster wbolster added the enhancement New feature or request label Aug 4, 2020
@wbolster wbolster self-assigned this Aug 4, 2020
@wbolster
Copy link
Owner Author

wbolster commented Aug 4, 2020

as follow-up to some comments from #26... what do you think of this, @pkryger?

update: this is already a bit outdated, very open to suggestions

image

(somehow hard to copy/paste the text from the popup, so here's an image instead)

@wbolster wbolster added the help wanted Extra attention is needed label Aug 4, 2020
@pkryger
Copy link
Contributor

pkryger commented Aug 5, 2020

I like the new layout with logical groping much better. (I also like the theme - a solarized-light user here ☀️).

I used a way to programmatically copy contents of *transient* buffer to a *scratch* and then copy text via clipboard. Thinking 🤔 of it now I should have probably been using (kill-new). Will find it and post later, when I get to a computer.

I've also noticed that in readme I put a code from a sample popup instead of the python-pytest-dispatch🤭. Will send a PR with an improved example.

@wbolster
Copy link
Owner Author

wbolster commented Aug 5, 2020

some questions/doubts/thoughts:

  • using multiple columns for flags/options? magit doesn't do it, but usually has fewer of them. i think it visually helps and since many of them are short it fits even on small screens, and saves a lot of vertical space

  • got rid of all key bindings starting with = since it's not required anymore, and some of the previous = flags didn't correspond to real pytest short options anyway.

  • use --foo options with double dash instead. some of them actually match (a prefix of) real pytest options (e.g. --lf), while others do not (--ft for --full-trace). is this too confusing or messy?

  • do the groupings make sense? maybe it can be reduced a bit?

  • should other options be added/removed?

@wbolster wbolster force-pushed the redesign-dispatch-menu branch from b073488 to 0c8d591 Compare August 5, 2020 08:22
@pkryger
Copy link
Contributor

pkryger commented Aug 5, 2020

Here's the trick to have a new kill item ready to yank rigth after invoking a popup/dispatch:

(define-advice python-pytest-dispatch (:after (&rest _))
  (with-current-buffer transient--buffer-name
    (copy-region-as-kill (point-min) (point-max))))
  • using multiple columns for flags/options? magit doesn't do it, but usually has fewer of them. i think it visually helps and since many of them are short it fits even on small screens, and saves a lot of vertical space

Same thoughts.

  • got rid of all key bindings starting with = since it's not required anymore, and some of the previous = flags didn't correspond to real pytest short options anyway.
  • use --foo options with double dash instead. some of them actually match (a prefix of) real pytest options (e.g. --lf), while others do not (--ft for --full-trace). is this too confusing or messy?

I like the idea of not having the = things anymore. The fact that some options double dash flags don't have corresponding pytest options is fine as well. As long as they don't step on any other pytest option I'm fine. I think that comes from the way I use transient: firstly it's a helper to explore options (with reasonable defaults), later it's muscle memory - I don't even read it - just blindly hit P -f p to push an amended commit 🤗. Not sure how open the pytest would be to accept these new switches 🤔

  • do the groupings make sense? maybe it can be reduced a bit?

Perhaps the Exit early could be merged into Test selection and ordering. The exit early could be interpreted as a dynamic filter 🤔

  • should other options be added/removed?

Not an expert here as I don't do much python 🤭😜. And IDK a good way of measuring it, unless you want to snoop on users' choices into some analytic cloud... but somehow I don't like that. Perhaps having a paragraph in a prominent place in the readme asking people to send their requests for missing options?

* use better groupings
* use multi-column visual layout
* add some more flags
* make all flags start with either ``-`` or ``--`` (mostly mimicking pytest flags)
@wbolster wbolster force-pushed the redesign-dispatch-menu branch from 0c8d591 to e2ff7e1 Compare August 10, 2020 14:00
@wbolster wbolster changed the title [wip] Redesign dispatch menu Redesign dispatch menu Aug 10, 2020
@wbolster
Copy link
Owner Author

wbolster commented Aug 10, 2020

i fiddled some more. this is what it looks like now:

Output
 -c color (--color)
 -q quiet (--quiet)
 -s no output capture (--capture=no)
 -v verbosity ([--verbose|--verbose --verbose])

Selection, filtering, ordering
 -k only names matching expression (-k=)      --dm run doctests (--doctest-modules)
 -m only marks matching expression (-m=)      --nf new first (--new-first)
                                              --sw stepwise (--stepwise)

Failures, errors, debugging
 -l show locals (--showlocals)                --ff failed first (--failed-first)
 -p debug on error (--pdb)                    --ft full tracebacks (--full-trace)
 -x exit after first failure (--exitfirst)    --mf exit after N failures or errors (--maxfail=10)
                                              --rx run xfail tests (--runxfail)
                                              --tb traceback style (--tb=)
                                              --tr debug on each test (--trace)

Run tests
 t all    r repeat         f file (dwim)    m files          d def/class (dwim)
          x last failed    F file (this)    M directories    D def/class (this)

pretty happy with it. unless @pkryger has some suggestions that need further work,
i'll likely merge this soon.

* origin/master:
  No longer use with-eval-after-load
  Get rid of python-pytest-arguments variable and helper function
@pkryger
Copy link
Contributor

pkryger commented Aug 10, 2020

IMO, it looks really good 👍

@wbolster wbolster merged commit 2c630e5 into master Aug 10, 2020
@wbolster wbolster deleted the redesign-dispatch-menu branch August 10, 2020 16:45
@wbolster
Copy link
Owner Author

cool, thanks for your review and previous input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants