Adds the ability to view the local filesystem from Bombadillo #45

sloum · 2019-10-05T20:29:26Z

sloum commented

2019-10-05 20:29:26 +00:00

I believe this solution should be pretty solid. The lack of readline for autocompletion makes it a little clunkier than people are used to for navigating a filesystem. However, it is functional and pretty neat I think. It is cool to be able to bookmark local files right next to gopher, gemini, telnet and web links.

I also set it up so that if the path that is entered leads to a directory then a simple directory listing is given.

This PR essentially lets Bombadillo function as a pager (though it does not accept pipes from stdout to stdin, you can pass it a local file as an argument and have it open to it).

The only bug I have found with it is that the "Loading..." message does not disappear when you pass in a filepath as an argument (though it does act correctly when you navigate to one from within Bombadillo). Not quite sure why this is happening. :-/

I believe this solution should be pretty solid. The lack of readline for autocompletion makes it a little clunkier than people are used to for navigating a filesystem. However, it is functional and pretty neat I think. It is cool to be able to bookmark local files right next to gopher, gemini, telnet and web links. I also set it up so that if the path that is entered leads to a directory then a simple directory listing is given. This PR essentially lets Bombadillo function as a pager (though it does not accept pipes from stdout to stdin, you can pass it a local file as an argument and have it open to it). The only bug I have found with it is that the "Loading..." message does not disappear when you pass in a filepath as an argument (though it does act correctly when you navigate to one from within Bombadillo). Not quite sure why this is happening. :-/

asdf was assigned by sloum

2019-10-05 20:29:26 +00:00

sloum self-assigned this 2019-10-05 20:29:26 +00:00

sloum added the

enhancement

label 2019-10-05 20:29:26 +00:00

asdf commented

2019-10-08 05:00:40 +00:00

This is another cool feature, would be useful for displaying a local help document too.

I can confirm the same issue with "Loading..." on my system. It goes away when you scroll down.

I my experience this is typically implemented as file://. There's no issue with local:// though, but I am curious why it was the chosen name?

Is it correct that this will just display the raw text with no interpretation? This makes sense, as gopher (and gemini?) communicate document type outside of the document itself. I'm used to web browsers interpreting HTML, but there is no way this could work when loading from the local file system with documents Bombadillo typically displays.

This is another cool feature, would be useful for displaying a local help document too. I can confirm the same issue with "Loading..." on my system. It goes away when you scroll down. I my experience this is typically implemented as *file://*. There's no issue with *local://* though, but I am curious why it was the chosen name? Is it correct that this will just display the raw text with no interpretation? This makes sense, as gopher (and gemini?) communicate document type outside of the document itself. I'm used to web browsers interpreting HTML, but there is no way this could work when loading from the local file system with documents Bombadillo typically displays.

sloum commented

2019-10-09 01:59:51 +00:00

Bummer about the "Loading..." situation. I spent more time than I would like to have spent trying to get that to not happen, but have not found the source of the issue. It does not happen when I pass a URL...

As for using local over file... partly I just wanted to do something different than a web browser, partly I wasnt sure what went into supporting file at a protocol level and did not want to have to put in a TON of work to this, and partly I always felt that file was confusing since all protocols serve up files of some sort...whereas local gives an indication of the source of the files.

Do you think it would be a good idea to change to file? I definitely can and do not have a major objection to doing so.

As to interpretation: not at present. Down the road we could add type sniffing and support syntax highlighting or some other features, but for now it seems like too much. My thought in adding it was that I sometimes have text files with info I need to reference from time to time and being able to bookmark them would be nice. I had not thought about opening a local gophermap or gemini document... the naming conventions for both are highly variable :-/ Maybe something to look into for 2.1.0. I think once this, the certificates for gemini, and finger get merged in that switching focus to bugs and docs is likely the good call. The feature set feels pretty robust at this point.

Bummer about the "Loading..." situation. I spent more time than I would like to have spent trying to get that to not happen, but have not found the source of the issue. It does not happen when I pass a URL... As for using local over file... partly I just wanted to do something different than a web browser, partly I wasnt sure what went into supporting `file` at a protocol level and did not want to have to put in a TON of work to this, and partly I always felt that file was confusing since all protocols serve up files of some sort...whereas local gives an indication of the source of the files. Do you think it would be a good idea to change to file? I definitely can and do not have a major objection to doing so. As to interpretation: not at present. Down the road we could add type sniffing and support syntax highlighting or some other features, but for now it seems like too much. My thought in adding it was that I sometimes have text files with info I need to reference from time to time and being able to bookmark them would be nice. I had not thought about opening a local gophermap or gemini document... the naming conventions for both are highly variable :-/ Maybe something to look into for 2.1.0. I think once this, the certificates for gemini, and finger get merged in that switching focus to bugs and docs is likely the good call. The feature set feels pretty robust at this point.

asdf commented

2019-10-09 23:14:23 +00:00

What a small and annoying bug. Debugging using delve would be great, but I think the standard debugging is incompatible with bombadillo because they both want to control drawing to the terminal. With ptrace enabled,it's possible to attach to a running process, but I haven't figured that part out.

I'm fine with using local:// as the URL, it does add some distinction.

Type sniffing is an interesting idea, I didn't think of that. I agree that it's not feasible for this PR though.

I'm ok with this merging in when you are ready.

What a small and annoying bug. Debugging using delve would be great, but I think the standard debugging is incompatible with bombadillo because they both want to control drawing to the terminal. With ptrace enabled,it's possible to attach to a running process, but I haven't figured that part out. I'm fine with using local:// as the URL, it does add some distinction. Type sniffing is an interesting idea, I didn't think of that. I agree that it's not feasible for this PR though. I'm ok with this merging in when you are ready.

sloum commented

2019-10-10 01:23:18 +00:00

I'll keep looking for a solution for that bug. I agree delve would be good. While it is not a debugger, setting up the log package to log to a file might be good... but would add a lot of code if we wanted a robust developer logging mode... which I am not excited about. Let me know if you manage to get delve working and how you do so!

Cool re: local. I'll stick with it for now, but we can always change later if it feels like the sensible thing to do.

Yeah, lets keep type sniffing in mind for the future. It may come in handy at some point. Probably not necessary for v2.0.0 though (maybe the next minor release).

👍 Merging in

I'll keep looking for a solution for that bug. I agree delve would be good. While it is not a debugger, setting up the log package to log to a file might be good... but would add a lot of code if we wanted a robust developer logging mode... which I am not excited about. Let me know if you manage to get delve working and how you do so! Cool re: local. I'll stick with it for now, but we can always change later if it feels like the sensible thing to do. Yeah, lets keep type sniffing in mind for the future. It may come in handy at some point. Probably not necessary for v2.0.0 though (maybe the next minor release). :thumbsup: Merging in

sloum closed this pull request

2019-10-10 01:23:22 +00:00

sloum deleted branch file-mode

2019-10-10 01:23:55 +00:00

Sign in to join this conversation.

No reviewers