blmayer / ereandel

A Gemini web browser using shell script
gemini://terminal.pink/
MIT License
56 stars 6 forks source link

Show link numbers and customize key bindings #35

Closed rockstorm101 closed 1 year ago

rockstorm101 commented 1 year ago

I guess this is a feature request, or I could not find a way to configure neither of these so please help:

  1. Customize key bindings while on less. I.e. make l open links instead of o.
  2. Display link numbers while "rendering" the pages. Like bollux does on the screenshot below. Links are clearly labelled 1 to 8 so I know quickly which one I want to follow when I hit g. Otherwise I have to read the list provided afterwards and guess which link is the one I wanted (which is easy when there's only 8 links but gets complicated when there's 50)

I don't really know how hard any of this is to implement but they are the only two things I missed when trying this software out. Nonetheless, great job on this little program, thank you for it.

2023-07-31_17-34

blmayer commented 1 year ago

Hi @rockstorm101, thank you for the suggestions. Answering your points:

Customize key bindings while on less. I.e. make l open links instead of o. I chose o so it comes from "Open", which is commonly used by browsers.

Display link numbers while "rendering" the pages. Like bollux does on the screenshot below. Links are clearly labelled 1 to 8 so I know quickly which one I want to follow when I hit g. Otherwise I have to read the list provided afterwards and guess which link is the one I wanted (which is easy when there's only 8 links but gets complicated when there's 50) I agree, it'd be great to have link numbers, I plan to implement it, but it's not my priority right now, but it should not be hard to implement. Also, I want to make paging using only shell functions, instead of relying on less.

Customization of keys is done on the script, it uses the lesskeys file to configure it, so it is not straight forward, you can customize by writing a less.keys file, base64 encoding it, and placing it on astro's config dir. You'll have to use the "extra string" configuration and set it to the previous letter, e.g.: to change o to l, use o as extra string. See https://man7.org/linux/man-pages/man1/lesskey.1.html.

When I implement the custom pager this will be a lot easier.

Give it a try, if it still doesn't work please let me know.

Nonetheless, great job on this little program, thank you for it. Thank you for the for the feedback and the suggestions.

rnwgnr commented 1 year ago

Regarding link counters: I've tried to implement this as a configurable option some time ago but failed (see discussion in https://github.com/blmayer/astro/pull/15).

For the time being i ended up with a personal soft-fork which uses hard coded link numbers (i.e. not configurable). See https://git.sr.ht/~rwa/astro/ for reference. I'd happily drop this fork if someone would make this configurable upstream. :)

blmayer commented 1 year ago

Hi, @rnwgnr, I took a look at your fork, the link numbers feature is working fine. If you want to add that feature here it is very welcome, and I think I can add some flags to make it configurable. What do you think?

rnwgnr commented 1 year ago

Sure, here we go: https://github.com/blmayer/astro/pull/37

rockstorm101 commented 1 year ago

Hi, I was actually in the middle to trying to implement this somehow. So here are my ideas in case it helps.

Code would look like:

line="${sty_linkb}${shp_linkb}${sty_linkt} ${line}"
line=$(echo "$line" | sed "s/%linkcounter/$linkcounter/g")
linkcounter=$((linkcounter + 1))

And in the configuration file you would have a line like any of this:

# Shape for link bullets
shp_linkb='=> [%linkcounter]'  # bollux style
# shp_linkb='%linkcounter>'    # René's style
# shp_linkb='=>'               # astro's style

Similarly you could allow customization for lists bullet points like:

# Symbol for lists bullet points
shp_listb='+'

Not 100% happy with the code since it involves a crude replacement of a literal string but it is the best I could come up with.

rockstorm101 commented 1 year ago

By the way, thank you both for your answers I got it working as I wanted to now.

When I implement the custom pager this will be a lot easier.

Not sure why you would need this? I would advise to keep everything as simple as possible (being dead simple is what brought me here in the first place) and that sounds a bit like reinventing the wheel.

rnwgnr commented 1 year ago

Would be nice if you'd submit a PR so Brian can have a look and decide whether to include it upstream.

Not sure if this a thing at all, but i was thinking about going over the code of astro and check if there are places where we can get rid of subshells (for improved performance).

blmayer commented 1 year ago

37 is merged, thanks @rnwgnr.

Not sure why you would need this? I would advise to keep everything as simple as possible (being dead simple is what brought me here in the first place) and that sounds a bit like reinventing the wheel.

Thanks for the heads up, if I ever implement a custom pager I would only do so if the result is simpler, and easier to understand, simplicity is the main goal here. Less is great but customizing it is hard and very limited, removing it could help in this, plus we will need one less dependency.

I plan to let the user use the $linkcounter variable in the config file, so it can be fully customized.

Not sure if this a thing at all, but i was thinking about going over the code of astro and check if there are places where we can get rid of subshells (for improved performance).

That's a neat idea.

blmayer commented 1 year ago

I will release a version now and start working on how to customize it.

rnwgnr commented 1 year ago

start working on how to customize it.

Looking forward to see your approach on this. I tried to do it without an additional replace-step back then and failed to get it working.

rnwgnr commented 1 year ago

Not sure if you are aware: commit d2e5597 breaks existing configs.

Do you plan to handle this or just advise users to re-configure?

blmayer commented 1 year ago

Yes, it breaks but I'm not sure if this is the only way. I think I could add new options to the config, instead of touching the previous ones, but this will increase the complexity.

Also, I failed to allow customization without using sed, sad. Been playing with eval, echo and '$' without success. I have to experiment a little more.

Thanks for the heads up.

rnwgnr commented 1 year ago

Also, I failed to allow customization without using sed, sad. Been playing with eval, echo and '$' without success. I have to experiment a little more.

Thats exactly the point where i failed as well some time ago. I didn't get it working without an additional string replacement which i wanted to avoid. So i ended up with hard coding the counter.

blmayer commented 1 year ago

I'm releasing the solution that needs a config rewrite. I'll warn users on the release description. Maybe in the future we can revisit this.

Also @rockstorm101 please check if you can customize keys now.

rockstorm101 commented 1 year ago

I'm releasing the solution that needs a config rewrite. I'll warn users on the release description. Maybe in the future we can revisit this.

Thank you for your work.

Also @rockstorm101 please check if you can customize keys now.

I can confirm key customization works with the latest code (297bdb0). Just needed to re-type the line on the new configuration file.

blmayer commented 1 year ago

Cool. Can I close this?

rockstorm101 commented 1 year ago

Cool. Can I close this?

Yes, I consider this matter closed. Many thanks.

For reference, issue addressed on #37 .

rockstorm101 commented 1 year ago

Closing.