bookmarks for search pages prompt the user to input the search term #29

New Issue

asdf · 2019-08-25T23:27:17Z

asdf commented

2019-08-25 23:27:17 +00:00

Hi Sloum!

Bookmarked search pages do not show the results, but instead prompt the user to perform another search. Pressing enter with no search then shows the bookmarked page.

I can reproduce this issue by doing the following:

Go to a page that is for searching things like gopher.floodgap.com:70/7/v2/vs
Enter a search term in the prompt
The search results are shown. Bookmark this page using :add . test
Open the bookmark again using :bookmarks n where n is the bookmark number

Instead of showing the results page, the user is prompted to perform the search again. Enter form input: is shown at the bottom of the screen.

Pressing enter with no text entry then brings up the expected page.

Hi Sloum! Bookmarked search pages do not show the results, but instead prompt the user to perform another search. Pressing enter with no search then shows the bookmarked page. I can reproduce this issue by doing the following: 1. Go to a page that is for searching things like `gopher.floodgap.com:70/7/v2/vs` 1. Enter a search term in the prompt 1. The search results are shown. Bookmark this page using `:add . test` 1. Open the bookmark again using `:bookmarks n` where n is the bookmark number Instead of showing the results page, the user is prompted to perform the search again. `Enter form input:` is shown at the bottom of the screen. Pressing `enter` with no text entry then brings up the expected page.

asdf commented

2019-08-25 23:52:38 +00:00

I took a look to try to understand this, but could not figure it out yet. Here's where I'm at so far:

Gopher has URLs of type 7 which are recognised as search items
The search URL should contain the page address, a tab, then the search term.
In main.go, when a search is identified, the URL is sent to the search function. This function will always prompt for search input.

I thought that modifying the search function to check for a URL containing a search term could fix this. Like:

func search(u string) error {
    if !strings.Contains(u, "	") {
        //prompt for search term, return searchurl
        ...
    } else {
        searchurl = u
}
// go to searchurl
...
}

This does not work though as the user is still prompted. I can try to get back to this later with a bit more time.

edit: this does actually work i was running the wrong binary lmao

I took a look to try to understand this, but could not figure it out yet. Here's where I'm at so far: - Gopher has URLs of type 7 which are recognised as search items - The search URL should contain the page address, a tab, then the search term. - In `main.go`, when a search is identified, the URL is sent to the `search` function. This function will always prompt for search input. I thought that modifying the search function to check for a URL containing a search term could fix this. Like: func search(u string) error { if !strings.Contains(u, " ") { //prompt for search term, return searchurl ... } else { searchurl = u } // go to searchurl ... } This does not work though as the user is still prompted. I can try to get back to this later with a bit more time. edit: this does actually work i was running the wrong binary lmao

sloum commented

2019-08-27 20:36:13 +00:00

lol, I run the wrong binary all the time! Yeah, you solved it more or less the way I was going to suggest (though in a slightly different place). I had taken a step back from this project since my development time is small and it has gotten complicated enough that I dont want to break something and donk things up for users and then not have time to fix it. That said, this issue seems very clearly fixable to me.

I see two paths:

Do as above and check for a tab and avoid prompting in that case. This has some flaws: more overhead, does not solve issues rendering plaintext as gophermap.
When a user submits a query, have the type overridden for the response to type 1. That way checks dont have to be done at all for querystrings, a search response would always return a gophermap... the only issue with this approach is that I do not believe all searches will return a gophermap. This will necessitate a slight change to gophermap rendering. I think this might be the better way to handle things, but I am not certain. Not knowing whether plaintext or a gophermap will be returned (or anything else) is one of the big issues with gopher as a protocol (one that project gemini is attempting to solve), and this falls into that territory.

I will try to start working on this asap, likely tomorrow. That said, if you'd rather have a go and want to submit a PR just let me know :)

lol, I run the wrong binary all the time! Yeah, you solved it more or less the way I was going to suggest (though in a slightly different place). I had taken a step back from this project since my development time is small and it has gotten complicated enough that I dont want to break something and donk things up for users and then not have time to fix it. That said, this issue seems very clearly fixable to me. I see two paths: 1. Do as above and check for a tab and avoid prompting in that case. This has some flaws: more overhead, does not solve issues rendering plaintext as gophermap. 2. When a user submits a query, have the type overridden for the response to type 1. That way checks dont have to be done at all for querystrings, a search response would always return a gophermap... the only issue with this approach is that I do not believe all searches will return a gophermap. This will necessitate a slight change to gophermap rendering. I think this might be the better way to handle things, but I am not certain. Not knowing whether plaintext or a gophermap will be returned (or anything else) is one of the big issues with gopher as a protocol (one that project gemini is attempting to solve), and this falls into that territory. I will try to start working on this asap, likely tomorrow. That said, if you'd rather have a go and want to submit a PR just let me know :)

sloum added the

bug

label 2019-08-27 20:36:24 +00:00

asdf commented

2019-08-28 05:29:04 +00:00

OK, I'm happy to try this, just want to clarify the detail as I'm learning all this as I go:

For a search in veronica2 with search term golang, the URL requested of the server is this:

gopher://gopher.floodgap.com:70/7/v2/vs(tab)golang

The suggestion is that the page showing results should be returned as type 1, meaning the URL displayed for it would become:

gopher://gopher.floodgap.com:70/1/v2/vs(tab)golang

If this page is bookmarked, the search results should be shown without a search prompt.

I tested by changing the bookmarks manually from type 7 to type 1. The pages display correctly and without a search prompt.

For reference, I had a look at other clients. gopher saves the bookmark as type 1, lynx saves it as type 7 but does not prompt, vf1 seems to have the same issue we are discussing.

OK, I'm happy to try this, just want to clarify the detail as I'm learning all this as I go: For a search in veronica2 with search term `golang`, the URL requested of the server is this: gopher://gopher.floodgap.com:70/7/v2/vs(tab)golang The suggestion is that the page showing results should be returned as type 1, meaning the URL displayed for it would become: gopher://gopher.floodgap.com:70/1/v2/vs(tab)golang If this page is bookmarked, the search results should be shown without a search prompt. I tested by changing the bookmarks manually from type 7 to type 1. The pages display correctly and without a search prompt. For reference, I had a look at other clients. `gopher` saves the bookmark as type 1, `lynx` saves it as type 7 but does not prompt, `vf1` seems to have the same issue we are discussing.

sloum commented

2019-08-28 14:47:41 +00:00

Yeah, you've got the gist of it. The only issue with changing it to type one is if the response is not a gophermap. I need to change how gophermaps are parsed to allow for plaintext lines. I can handle that part if you want to handle the other :)

sloum commented

2019-08-29 23:59:15 +00:00

@asdf I had some time today and worked through a version of how this could be handled. I am still very open to a solution of yours and would love to see a PR if you started on a solution. I have created a PR ( #30 ) in order to get comments from you, or anyone else, regarding this issue. I have tested the code in a local build and it seems to solve the issue pretty well.

Let me know if you'd like me to proceed with merging that PR in or if you will have one forthcoming or if you have concerns or comments. As always, thanks for contributing!

@asdf I had some time today and worked through a version of how this could be handled. I am still very open to a solution of yours and would love to see a PR if you started on a solution. I have created a PR ( https://tildegit.org/sloum/bombadillo/pulls/30 ) in order to get comments from you, or anyone else, regarding this issue. I have tested the code in a local build and it seems to solve the issue pretty well. Let me know if you'd like me to proceed with merging that PR in or if you will have one forthcoming or if you have concerns or comments. As always, thanks for contributing!

sloum referenced this issue from a commit

2019-09-01 02:54:31 +00:00

Merge branch 'type-seven-bm-fix' of sloum/bombadillo into master Thanks for all the help @asdf! Merging this into master. This PR closes issue #29

sloum commented

2019-09-01 02:55:36 +00:00

Closed by #30

Closed by https://tildegit.org/sloum/bombadillo/pulls/30

sloum closed this issue

2019-09-01 02:55:36 +00:00

Sign in to join this conversation.