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

fix for #103: Simple handling of local (file://) urls #117

Merged
merged 24 commits into from
Nov 22, 2020

Conversation

jedthehumanoid
Copy link
Contributor

@jedthehumanoid jedthehumanoid commented Nov 8, 2020

Simple implementation for rendering local (file://) documents.

When opening a file:// url:
If it is a directory:
Render a page with links to all dirs and files in directory.
Links are made to all kinds of files, checking if reasonable to open is done when actually opening them (with error popups)

If it is a file:

  • If not a gemini file, use mime.TypeByExtension to check if text/* otherwise bail
  • render .gemini/.gmi as gemini
  • else render as plaintext

TODO

  • directory listing
  • Files
    • prohibit binaries
    • gemini files
    • other plain text files
  • links to other local resources
  • links to non-file://urls works as usual
  • Test on Linux
  • Test on Windows

fixes #103

@makew0rld
Copy link
Owner

Thanks for your work!

I see this PR is marked as a draft, so I won't look at the code yet. But to address your questions:

Showing .txt files as well is straigtforward, but in that case we could show all sensible plain-text files regardless of extension, with some simple heuristic to ignore binary files and files too large.

That sounds good to me, I would like plain text support to be added. If that heuristic you mention ends up being too complicated or intensive, that just looking for *.txt is good with me too.

Does not show directory listings for now. AFAICT normal gemini:// resouces does not do that either, so I am unsure about a idiomatic solution for doing this. Render directories as links and parsing it as a gemini-document?

Normal gemini sites aren't required to show directory listings, but most servers will if an index.gmi/gemini file doesn't exist. I would prefer it as it makes finding the file you want easier. And rendering directories as links is the way to go, yes. And I was picturing that only the files Amfora could open would be listed, so no .jpg or anything.

Links to online resources works fine, but links to local files is wonky right now.

Linking to relative files can be fixed by creating a handleFile func, that contains most of the code you've added. Then in handleURL, make use of handleFile for two situations:

  • When the URL starts with file://
  • When the page/tab URL starts with file:// and it's a local link

This should fix linking to local files.

@jedthehumanoid
Copy link
Contributor Author

Thanks for taking the time to investigate!

Yeah I wanted to ventilate alternatives before continuing.

Will give it a shot as discussed

  • Show all plain-text files if not to complicated to evaluate if truly plain-text
  • Render and show directory listing
  • Fix local linking

@makew0rld
Copy link
Owner

Thanks! Please mark this as not-WIP when you're done and make a comment so I get notified.

@jedthehumanoid jedthehumanoid marked this pull request as ready for review November 14, 2020 13:01
@jedthehumanoid
Copy link
Contributor Author

@makeworld-the-better-one ok, tested on Windows as well as Linux

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 a lot for adding this! It's a great feature to have. Please see my comments below.

Also, it would be helpful to test with directories and files with Unicode characters in them. Please also test the directory view with those.

@jedthehumanoid
Copy link
Contributor Author

Yes, also works with utf8 filenames and directories, just tested with my national åäö's, and also various wonky faces and symbols 😎

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 updating the PR! It's almost ready, there's just one bug left, and some cosmetic changes.

@makew0rld makew0rld merged commit 51fd32e into makew0rld:master Nov 22, 2020
makew0rld added a commit that referenced this pull request Nov 22, 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.

Open Local Gemini Files
2 participants