makew0rld / amfora

A fancy terminal browser for the Gemini protocol.
GNU General Public License v3.0
1.17k stars 67 forks source link

Table of Contents #182

Open ThomasAdam opened 3 years ago

ThomasAdam commented 3 years ago

Hey @makeworld-the-better-one

As others have echoed many times, thanks for this client -- it's certainly one of the better terminal ones!

I was looking at the TODO list, and noticed you had an item on there which I'm keen to work on, assuming it's not already being implemented elsewhere (in which case I'll look at some other feature to code), and that's adding a ToC (Table of Contents).

In adding this, I have some questions before I start submitting PRs for this. The first one is to do with the parser (in renderer.go). At the moment, the parser is stateless in that it's rendering the page line-by-line, and building up a display output which can be rendered. I'm actually wondering if we should instead have the concept of a Parser, perhaps something like:

type Parser struct {
    scanner           *bufio.Scanner
    Lines             []Line
    isPre             bool
    tableofcontents   []*Heading // see toc.go
}

Where we have a bufio.Scanner representing the raw gemini lines, and that Lines is out struct of parsed content.

From there, we can introduce further types that know how to handle:

Most of this is yak-shaving to then be able to build up a ToC though, which is my next point. As I see it, a ToC would have to represent two things:

  1. An overview of the document, with backlinks to the rendered document so that it's scrolled at the appropriate point (think HTML anchors foo.html#whatever)
  2. Be displayed somewhere convenient on the UI

Did you have anything in mind with regards to point 2? I was imagining a sidebar on either the left or right of the screen which can be toggled on and off, perhaps?

Point 1 is just a variation on a theme about getting the Parser into a shape where adding in these ephemeral states which fall outside of pure gemini-rendering become easier.

Let me know your thoughts, and I'll happily crack on with this.

Thanks again! Thomas

makew0rld commented 3 years ago

Just a heads up, I edited your parser code example to make it more correct and readable, feel free to change it if I made a mistake.

Thanks for the compliment, and I'm happy to hear you're interested in working on this feature!

I'm actually wondering if we should instead have the concept of a Parser

Definitely. I find the current method of rendering somewhat ugly and hacky, and more importantly, it requires the entire document being available, but gemtext is line-based. This is actually quite related to the idea of streaming gemtext responses (#9), which would require a parser. In this comment I suggested a parser that implemented io.Writer. The idea was that a network request would write the data it read into the parser, which would be associated with a specific cview.TextView, and write the rendered output there.

Of course there are other ways to do this, like to have the network connection as a field of the parser, and then the parser reads and renders in a goroutine or something. I'm not sure what's the best method.

Now all of this is somewhat separate from your original idea of building a ToC feature. I've brought it up because it would be a good thing to work on if it simplifies or helps things. If it's too much extra work to close #9 then don't worry about it, any parser you write will be helpful in moving towards that goal anyway.

An overview of the document, with backlinks to the rendered document so that it's scrolled at the appropriate point (think HTML anchors foo.html#whatever)

Yep! Although I think we should stay away from actually using anchors for now, as that's not standardized in Gemini yet. cview.TextView has the ScrollTo(row, column int) method that will work for this. The tab struct also has a method for saving the scroll after you change it that you'll want to use, this will save scroll when moving through history and other things.

So the table of contents data will need to contain heading lines, but it will also need to know what line of the document that heading is. I'm not sure if there should be a separate table of contents struct, or it can be built or stored with the Parser?

Be displayed somewhere convenient on the UI

This feels like the trickier one to me, space is limited.

Did you have anything in mind with regards to point 2? I was imagining a sidebar on either the left or right of the screen which can be toggled on and off, perhaps?

This is what I picture as well. I would use the right side, as Amfora is designed to have a larger right margin then left margin on larger screens. You'll need to create a cview Primitive that holds the ToC text, then include it in the cview.Flex, and figure out how to get the dynamic resizing to work right. Make sure to test with a terminal window, and change the size and see how it responds.

You may want to use a cview.TreeView to hold the ToC text. Here's some TreeView demo code.

You should be basing your work off the cview-update branch until #107 is merged, as that branch represents the next iteration of Amfora, and uses the newer cview version which is not fully compatible with the version currently used in master.

I will "assign" you this issue in GitHub's interface. If you can't work on it anymore in the future for whatever reason, just let me know and I'll unassign you.

makew0rld commented 3 years ago

@ThomasAdam I see you've begun this work here. Have you made further private progress on this? I ask because I am going to be working on #9 next, and that will mean implementing the things we discussed above like a parser, everything short of an actual ToC feature. If you're started or completed this yourself I'd be happy to see that work and possibly use it, otherwise I'll start on it myself and you can continue your ToC work once I've finished.

Thanks!

ThomasAdam commented 3 years ago

Hi @makeworld-the-better-one,

Apologies for the delay. You're working a lot faster than I am. Please go ahead with #9 and I'll both review that and complete the ToC work.

makew0rld commented 3 years ago

All good, thanks for getting back to me. And yeah, unfortunately it might make sense to hold off on this until #9 is completed because of how related they are.

makew0rld commented 2 years ago

These issues are related, as ideally they require parsing of headers to work fully: #202 #267 #286