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

Help the developer when they specify an invalid filter by providing some... #549

Merged
merged 2 commits into from Dec 7, 2011

Conversation

jonathaningram
Copy link
Contributor

... alternatives.

Bug fix: no
Feature addition: yes
Backwards compatibility break: no

Uses a simple strpos search (no fancy diff to determine what the filter could be). There is no impact to runtime performance because this is all happening during template compilation.

Examples:

{{ my_entity|json }}

Shows exception:

The filter "json" does not exist. Did you mean "json_encode"?

{{ my_date|dat }}

Shows:

The filter "dat" does not exist. Did you mean "date"?

{{ my_var|f }}

Shows:

The filter "f" does not exist. Did you mean "format", "default", "_default", "format_args", "format_args_as_text", "file_excerpt", "format_file", "format_file_from_text", "file_link"?

Also good for custom filters created in your own domain st_:

{{ my_var|st_ }}

Shows:

The filter "st_" does not exist. Did you mean "st_friendly_date", "st_site_test_url"?

@stof
Copy link
Member

stof commented Dec 5, 2011

I like the idea to provide help to the developer in case of error. You should probably do the same for functions

@@ -19,8 +19,23 @@ public function __construct(Twig_NodeInterface $node, Twig_Node_Expression_Const
public function compile(Twig_Compiler $compiler)
{
$name = $this->getNode('filter')->getAttribute('value');

Copy link
Member

Choose a reason for hiding this comment

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

please remove all trailing whitespaces

if (false !== strpos($filterName, $name)) {
$alternativeFilters[] = $filterName;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to use levenshtein too:

<?php
foreach ($compiler->getEnvironment()->getFilters() as $filterName => $filter) {
    $diffs = levenshtein($filterName, $name);
    if ($diffs <= strlen($name)/3) {
         $alternativeFilters[$filterName] = $diffs;
    }
    if (false !== strpos($filterName, $name)) {
        $alternativeFilters[$filterName] = $diffs;
    }
}
asort($alternativeFilters);
$alternativeFilters = array_keys($alternativeFilters);

(untested)

This will suggest filters whose name is less than 1/3 different from the entered name; and will sort alternative filters by the number of differences.

Copy link
Contributor

Choose a reason for hiding this comment

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

levenshtein is indeed what we need here. That's what I'm using for the Twig website when you search for an unknown filter/tag/function and it works very well.

@jonathaningram: Can you update your PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabpot I will. Any chance of posting the levenshtein code snippet from the Twig website and I'll update the PR directly? No need to reinvent the wheel.

@Tobion
Copy link
Contributor

Tobion commented Dec 5, 2011

+1 for a combination of string starts with (suggesting domain specific filters, e.g. st_*) and levenshtein for typos

@chucktrukk
Copy link

+1. This would be really helpful.

fabpot added a commit that referenced this pull request Dec 7, 2011
Commits
-------

989bb22 Removed trailing white spaces
f880bab Help the developer when they specify an invalid filter by providing some alternatives

Discussion
----------

Help the developer when they specify an invalid filter by providing some...

... alternatives.

Bug fix: no
Feature addition: yes
Backwards compatibility break: no

Uses a simple `strpos` search (no fancy diff to determine what the filter could be). There is no impact to runtime performance because this is all happening during template compilation.

Examples:

```jinja
{{ my_entity|json }}
```

Shows exception:

`The filter "json" does not exist. Did you mean "json_encode"?`

```jinja
{{ my_date|dat }}
```

Shows:

`The filter "dat" does not exist. Did you mean "date"?`

```jinja
{{ my_var|f }}
```

Shows:

`The filter "f" does not exist. Did you mean "format", "default", "_default", "format_args", "format_args_as_text", "file_excerpt", "format_file", "format_file_from_text", "file_link"?`

Also good for custom filters created in your own domain `st_`:

```jinja
{{ my_var|st_ }}
```

Shows:

`The filter "st_" does not exist. Did you mean "st_friendly_date", "st_site_test_url"?`

---------------------------------------------------------------------------

by stof at 2011/12/04 16:34:27 -0800

I like the idea to provide help to the developer in case of error. You should probably do the same for functions

---------------------------------------------------------------------------

by Tobion at 2011/12/05 05:56:29 -0800

+1 for a combination of string starts with (suggesting domain specific filters, e.g. st_*) and levenshtein for typos

---------------------------------------------------------------------------

by chucktrukk at 2011/12/05 08:13:10 -0800

+1. This would be really helpful.
@fabpot fabpot merged commit 989bb22 into twigphp:master Dec 7, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants