jbmestelan / nyxt

Nyxt - Be productive.
https://nyxt.atlas.engineer
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Sweep: SEARCH-ENGINES: SEARCH-URL fails to load on an empty search #5

Open jbmestelan opened 1 year ago

jbmestelan commented 1 year ago

The documentation of the SEARCH-ENGINES function mentions:

If the query is empty, FALLBACK-URL is loaded instead. If FALLBACK-URL is empty, SEARCH-URL is used on an empty search.

However, in the case where FALLBACK-URL is empty, I find the following: on an empty query, we only get a blank page. The log then mentions "Loading "about:blank".

Checklist - [X] `source/search-engine.lisp` > • Modify the `fallback-url` method to check if the `fallback-url` is not only `nil` but also an empty string. If it is, return the `search-url` with an empty string. > • In the `make-search-engine` function, add a check to ensure that if the `fallback-url` is an empty string, it is set to `nil`. - [X] `source/urls.lisp` > • Modify the `valid-url-p` function to return true if the URL is a `search-url` with an empty string. This can be done by adding a condition to check if the URL is a `search-url` and if the query string is empty.
sweep-ai[bot] commented 1 year ago

Here's the PR! https://github.com/jbmestelan/nyxt/pull/6.

⚡ Sweep Free Trial: I used GPT-4 to create this ticket. You have 4 GPT-4 tickets left for the month and 1 for the day. For more GPT-4 tickets, visit our payment portal. To retrigger Sweep, edit the issue.


Step 1: 🔍 Code Search

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description. https://github.com/jbmestelan/nyxt/blob/c0e1ca52bf2068e5e448b5e54cdbc8161c505303/source/search-engine.lisp#L1-L151 https://github.com/jbmestelan/nyxt/blob/c0e1ca52bf2068e5e448b5e54cdbc8161c505303/source/urls.lisp#L206-L327 https://github.com/jbmestelan/nyxt/blob/c0e1ca52bf2068e5e448b5e54cdbc8161c505303/source/mode/small-web.lisp#L157-L251 https://github.com/jbmestelan/nyxt/blob/c0e1ca52bf2068e5e448b5e54cdbc8161c505303/libraries/web-extensions/browser.c#L14-L71 https://github.com/jbmestelan/nyxt/blob/c0e1ca52bf2068e5e448b5e54cdbc8161c505303/source/web-extensions.lisp#L1-L57

Step 2: 🧐 Snippet Analysis

From looking through the relevant snippets, I decided to make the following modifications:

File Path Proposed Changes
source/search-engine.lisp Modify source/search-engine.lisp with contents:
• Modify the fallback-url method to check if the fallback-url is not only nil but also an empty string. If it is, return the search-url with an empty string.
• In the make-search-engine function, add a check to ensure that if the fallback-url is an empty string, it is set to nil.
source/urls.lisp Modify source/urls.lisp with contents:
• Modify the valid-url-p function to return true if the URL is a search-url with an empty string. This can be done by adding a condition to check if the URL is a search-url and if the query string is empty.

Step 3: 📝 Planning

I have created a plan for writing the pull request. I am now working my plan and coding the required changes to address this issue. Here is the planned pull request:

Fix issue with empty fallback-url in search-engine sweep/fix-empty-fallback-url

Description

This PR fixes an issue where the fallback-url in the search-engine class does not correctly handle the case when it is empty. According to the documentation, if the fallback-url is empty, the search-url should be used on an empty search query. However, the current implementation results in a blank page being loaded instead. This PR addresses this issue by modifying the fallback-url method and the valid-url-p function to handle the empty fallback-url case correctly.

Summary of Changes

  • Modified the fallback-url method in source/search-engine.lisp to check if the fallback-url is an empty string. If it is, the search-url with an empty string is returned.
  • Modified the make-search-engine function in source/search-engine.lisp to set the fallback-url to nil if it is an empty string.
  • Modified the valid-url-p function in source/urls.lisp to return true if the URL is a search-url with an empty string.

This PR ensures that the search-url is correctly used when the fallback-url is empty, preventing a blank page from being loaded.


Step 4: ⌨️ Coding

File Instructions Progress Error logs
source/search-engine.lisp Modify source/search-engine.lisp with contents:
• Modify the fallback-url method to check if the fallback-url is not only nil but also an empty string. If it is, return the search-url with an empty string.
• In the make-search-engine function, add a check to ensure that if the fallback-url is an empty string, it is set to nil.
✅ Commit d27d1f3 No errors.
source/urls.lisp Modify source/urls.lisp with contents:
• Modify the valid-url-p function to return true if the URL is a search-url with an empty string. This can be done by adding a condition to check if the URL is a search-url and if the query string is empty.
✅ Commit e1d3c0a No errors. I have finished coding the issue. I am now reviewing it for completeness.

Step 5: 🔁 Code Review

Here are my self-reviews of my changes at sweep/fix-empty-fallback-url.

Here is the 1st review

No changes required. The modifications made in both source/search-engine.lisp and source/urls.lisp are logically sound and correctly implemented. They successfully handle the case where 'fallback-url' is an empty string and where a 'search-url' has an empty string. Good job!

I finished incorporating these changes.


🎉 Latest improvements to Sweep:


💡 To recreate the pull request edit the issue title or description. Join Our Discord