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

WebSuggest: offer default suggestion #4

Merged
merged 2 commits into from
Aug 26, 2017
Merged

WebSuggest: offer default suggestion #4

merged 2 commits into from
Aug 26, 2017

Conversation

alexandr-san4ez
Copy link
Contributor

Note: if arguments empty - open the search home page.

@polyvertex polyvertex changed the title Added default item if not founded suggestions and waiting parameter WebSuggest: offer default suggestion Aug 12, 2017
Copy link
Member

@polyvertex polyvertex left a comment

Choose a reason for hiding this comment

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

Thanks for that! Nicely blends with existing code

@@ -37,6 +37,12 @@
# * Default: yes
#enable_predefined_items = yes

# Time that the plugin will wait before sending the request.
# * Time in seconds (can be used with float type)
# * The minimum value is 0.25
Copy link
Member

Choose a reason for hiding this comment

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

Please specify a hard-coded maximum value to prevent mistyping. 3 seconds should do it I think

# avoid doing unnecessary network requests in case user is still typing
if len(user_input) < 2 or self.should_terminate(0.25):
if len(user_input) < 2 or self.should_terminate(self.waiting_time):
self.set_suggestions(suggestions)
Copy link
Member

Choose a reason for hiding this comment

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

set_suggestions() call is useless here since suggestions will be discarded by KP anyways

Copy link
Contributor Author

@alexandr-san4ez alexandr-san4ez Aug 12, 2017

Choose a reason for hiding this comment

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

This is for the case of entering one character. But I agree that this is unnecessary. Nobody is looking for 1 character. :)

self.set_suggestions(suggestions, kp.Match.ANY, kp.Sort.NONE)
if not provider_suggestions: # change default item
suggestions[0].set_short_desc("No suggestions found (default action: {})".format(
profile['default_action']))
Copy link
Member

Choose a reason for hiding this comment

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

Please indent to align with above parenthesis. profile[... should be aligned with "No suggestions ... (I know it may not be the case everywhere in the repo)

@@ -405,6 +424,9 @@ def _read_config(self):
enable_predefined_items = settings.get_bool(
"enable_predefined_items", self.CONFIG_SECTION_MAIN,
fallback=self.DEFAULT_ENABLE_PREDEFINED_ITEMS)
self.waiting_time = settings.get_float(
"waiting_time", self.CONFIG_SECTION_MAIN,
fallback=self.DEFAULT_WAITING_TIME, min=0.25)
Copy link
Member

Choose a reason for hiding this comment

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

(max value)

# * Time in seconds (can be used with float type)
# * The minimum value is 0.25
# * Default: 0.25
#waiting_time = 0.25
Copy link
Member

Choose a reason for hiding this comment

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

I think idle_time is more appropriate than waiting_time

@polyvertex
Copy link
Member

Please PR on the dev branch

@alexandr-san4ez alexandr-san4ez changed the base branch from master to dev August 12, 2017 10:20
@polyvertex
Copy link
Member

Looks good. Thanks again!

@polyvertex polyvertex merged commit 7707989 into Keypirinha:dev Aug 26, 2017
@alexandr-san4ez alexandr-san4ez deleted the web-suggest branch August 27, 2017 10:32
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.

2 participants