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

Custom new tab page #83

Merged
merged 9 commits into from
Sep 1, 2020
Merged

Custom new tab page #83

merged 9 commits into from
Sep 1, 2020

Conversation

sotpapathe
Copy link
Contributor

The new tab page can now be loaded from $XDG_CONFIG_HOME/amfora/new_tab.gmi. This is documented in the default tab page. This implements what was suggested in issue #67 except for the ability to reload the new tab page, it is loaded once on startup.

  • I didn't make the new tab file location configurable. It will require expanding ~ in the config option and possibly environment variables in order to honor XDG_CONFIG_HOME so I opted for a hardcoded path like the config file. If it is deemed necessary I can implement it.
  • config.ConfigDir was exported to allow accessing XDG_CONFIG_HOME from outside config.
  • I didn't implement the reload yet because it will require some special cases and I'm not sure where best to add them, Reload(), or hasContent() and handleURL()?

It makes it possible to store other kinds of configuration files in it, such as
a custom new tab page.
Copy link
Owner

@makew0rld makew0rld 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 adding this! I have some comments.

  • Don't export configDir, but instead have a new exported variable called NewTabPath. The config package can do the filepath.Join(config.ConfigDir, "new_tab.gmi") work.
  • Use newtab.gmi, not new_tab.gmi

I didn't make the new tab file location configurable.

All good with me.

I didn't implement the reload yet because it will require some special cases and I'm not sure where best to add them, Reload(), or hasContent() and handleURL()?

I would add a config boolean called CustomNewTab, set on startup. Then in Reload, I would add code like this:

func Reload() {
	if tabs[curTab].page.URL == "about:newtab" && config.CustomNewTab {
		// Re-render new tab, similar to Init()
		newTabContent := getNewTabContent()
		tmpTermW := termW
		renderedNewTabContent, newTabLinks = renderer.RenderGemini(newTabContent, textWidth(), leftMargin())
		newTabPage = structs.Page{
			Raw:       newTabContent,
			Content:   renderedNewTabContent,
			Links:     newTabLinks,
			URL:       "about:newtab",
			Width:     tmpTermW, // Note this line is different than from Init(), since the terminal width is known
			Mediatype: structs.TextGemini,
		}
	}

    if !tabs[curTab].hasContent() {
        return
    }
	
	// Reload func continues...
}

display/newtab.go Outdated Show resolved Hide resolved
display/newtab.go Outdated Show resolved Hide resolved
@sotpapathe
Copy link
Contributor Author

Done! I didn't add the CustomNewTab boolean because if the custom new tab page is created after startup, the custom page won't appear after reloading. With the current implementation, at each reload, a custom new tab page is searched for.

@makew0rld
Copy link
Owner

makew0rld commented Sep 1, 2020

Thanks! The problem with not having CustomNewTab is that if there is no new tab page, the the default new tab is still re-rendered every time you open a new tab, even if the terminal is the same size.

if the custom new tab page is created after startup, the custom page won't appear after reloading

I'm okay with this, because of the performance gain.

Once this is added, I will be happy to merge.

@sotpapathe
Copy link
Contributor Author

Fair point, done. If a custom new tab page is present on startup, then the new tab page will always be reloaded. Otherwise it will never be reloaded, even if a custom new tab page appears later.

Should I squash my commits before merging?

@makew0rld
Copy link
Owner

Thanks for all your work! Merged. And no, you don't have to squash, I can do that with the "Squash and merge" button.

@makew0rld makew0rld merged commit 9d7c737 into makew0rld:master Sep 1, 2020
@makew0rld makew0rld mentioned this pull request Sep 1, 2020
@sotpapathe
Copy link
Contributor Author

Got it. Thanks for merging and making amfora!

@sotpapathe sotpapathe deleted the custom-new-tab-page branch September 1, 2020 17:59
makew0rld added a commit that referenced this pull request Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants