Added ability to save about: pages.
#236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
makew0rld
requested changes
May 13, 2021
display/display.go
Outdated
| @@ -576,7 +576,7 @@ func Reload() { | |||
| return | |||
| } | |||
|
|
|||
| if !tabs[curTab].hasContent() { | |||
| if !tabs[curTab].hasContent() || tabs[curTab].isAnAboutPage() { | |||
There was a problem hiding this comment.
Actually, reloading about pages should be allowed, so this can be removed.
display/download.go
Outdated
| parsed, _ := url.Parse(u) | ||
| if parsed.Path == "" || path.Base(parsed.Path) == "/" { | ||
| if strings.HasPrefix(parsed.String(), "about:") { |
There was a problem hiding this comment.
Suggested change
| if strings.HasPrefix(parsed.String(), "about:") { | |
| if strings.HasPrefix(u, "about:") { |
display/download.go
Outdated
Comment on lines
327
to
328
| // Is an about page, use the entire page url since there is no hostname | ||
| name, err = getSafeDownloadName(dir, parsed.String()+ext, true, 0) |
There was a problem hiding this comment.
I would just use the "path" or whatever. Like about:subscriptions should be saved as subscriptions.gmi.
|
Thanks for the fix! Note I made a small change in 56b7e4a, because otherwise URLs like Otherwise looks good, thanks for your work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, I've been using Amfora for short-while and wanted to contribute to this awesome project.
Proposed Changes:
hasContentto its own separate function as that was the only check prohibiting about page saves. Since a page's content has no implication on it being an about page, decoupling these checks seems like a good idea. But do let me know if this change does not make sense for the overall project, and I can look into other fixes.downloadNameFromURLso that when it's generating a filename for a saved about page, it uses the entire page url i.e.about:subscriptionsas otherwise the filename was simply left blank.Fixes #210
P.S. I did not locate a contribution guide, so let me know if I went about something the wrong way. If there is such a guide, please do point me towards it.