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

MINOR: adding filter variable for dev/tasks #10242

Closed

Conversation

sunnysideup
Copy link
Contributor

Long overdue change to find tasks easily.

Tasks can be filtered by adding the q request variable (e.g. /dev/tasks/?q=test OR vendor/bin/sake dev/tasks q=test)

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Is there an easy way to implement a small filter form on the page? Otherwise I feel like this is likely to go unnoticed by people who would otherwise find it useful.

I'm assuming this is mostly for use in the browser, since getting tasks with sake you can just pipe into grep for the same functionality.

$tasks = $this->getTasks();
$filter = (string) trim($request->requestVar('q'));
if($filter) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if($filter) {
if ($filter) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

$tasks,
function ($v) use ($filter) {
$t = $v['title'] ?? '';
$d = $v['description'] ?? '';
Copy link
Member

Choose a reason for hiding this comment

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

The $v, $d, and $t variables should probably be more verbose. I'm not even sure what $v is short for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@sunnysideup
Copy link
Contributor Author

I'm assuming this is mostly for use in the browser, since getting tasks with sake you can just pipe into grep for the same functionality.

Not from my point of view. This requires you to know how to use grep, etc... I would keep it simpler (or provide some information on how to use grep).

$tasks = $this->getTasks();
$filter = (string) trim($request->requestVar('q'));
Copy link
Contributor

Choose a reason for hiding this comment

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

trim already casts to string, so the extra casting is not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true!

@sunnysideup
Copy link
Contributor Author

If you guys like it then I will make a new pull request with everything sorted. I am not sure about the form though as we dont want to make it too fancy. Just simple.

@sunnysideup
Copy link
Contributor Author

Maybe we can add a little form, but then I think this should be Javascript based, not PHP based.

{
$baseUrl = Director::absoluteBaseURL();
Copy link
Contributor

Choose a reason for hiding this comment

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

This line does nothing here by the looks, is it needed?

@michalkleiner
Copy link
Contributor

My 2c — even though it's quite a small addition, I personally think it's not a scope we want to support within the framework in the long term, mainly because there are other quite standard ways how to achieve the same without this — as Guy mentioned, in the CLI folks can use grep or other filtering mechanisms, in the browser you can search within the content of the page using CTLR/CMD+F shortcut.

@sunnysideup
Copy link
Contributor Author

Yeah - then lets leave it. Dont you think? I guess what would be awesome if we had some sort of autocomplete. I usually use CTRL+F on the screen, but most of the time I run it from command line and then I just copy the output and do a CTRL+F.

Maybe it is better to have the tasks grouped or something like that as we work on lots of projects with 50 or so tasks - where it is a real pain to find the right one.

@GuySartorelli
Copy link
Member

GuySartorelli commented Apr 12, 2022

Yeah - then lets leave it.

Closing this PR

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

3 participants