search

pop-os / launcher

Modular IPC-based desktop launcher service
Mozilla Public License 2.0
232 stars 48 forks source link

Add Brave search to config.ron #138

Closed ghost closed 2 years ago

jacobgkau commented 2 years ago

@n3m0-22 That's a good concern, but doesn't the search option get pushed to the top when you add the query?

https://user-images.githubusercontent.com/7199422/187233368-2c850f6b-a63f-4e3f-a362-f556a6e6210b.mp4

This seems like sane behavior to me.

n3m0-22 commented 2 years ago

Yes it is pushed to the top when the query is entered. My thinking was if this is being added to launcher to make for faster brave searches having a secondary match that starts at the top preventing accidentally opening a new tab would be useful.

jacobgkau commented 2 years ago

having a secondary match that starts at the top preventing accidentally opening a new tab would be useful.

Pressing Enter on the search Launcher entry without entering a query opens a new tab anyway, so I don't see how that prevents accidentally opening a new tab.

I'm not entirely against the idea of a shorter prefix option (Google and Yahoo have two-character options in addition to their full name), just questioning the necessity. Google has gs, so I could see bs to specify Brave Search as opposed to Brave Browser (or your suggestion of brv would also work.)

n3m0-22 commented 2 years ago

You're correct in that instance it wouldn't. I think bs is probably a better option over brv for consistency.

questioning the necessity

It is not necessary, but it seems to me that if you are using launcher for any search engine you're doing it for speed and convince, so a secondary shorthand option would make sense.

ghost commented 2 years ago

I agree. New commit uses bs, and the name is changed from Brave to Brave search to distinguish the search from the app.

jacobgkau commented 2 years ago

Brave Search is the full product name, so I capitalized the second word.

I also added the longer brave prefix back as an alternative to bs, again following what we're doing for other search engines. We can remove it if Brave Browser users have problems with it later, but as of right now, the result sorting handles it correctly (and some users may be using Brave Search without having Brave Browser installed, so not having it as an easier-to-discover/easier-to-remember option seems more opaque than necessary.)

jacobgkau commented 2 years ago

@causality-loop Please keep your fork around until after a PR's been merged in the future! This PR is still valid, but we wouldn't be able to make any further changes now that the branch is gone.

ghost commented 2 years ago

@jacobgkau Whoops. Sorry, this is my first time contributing to a "legit" project on GH. Thank you for putting up with my stupidity. New PR created: web: add brave search to config.ron #139 and closing this one.