when downloading a file, an absolute file path is not supported #75

New Issue

asdf · 2019-11-04T02:23:45Z

asdf commented

2019-11-04 02:23:45 +00:00

When saving a file, entering the command to save to a specific location will usually not work, as paths are always built relative to the download path.

For example, saving a file using the command w . ~/test will not work, as Bombadillo tries to write to downloadpath/~/test.

When saving a file, entering the command to save to a specific location will usually not work, as paths are always built relative to the download path. For example, saving a file using the command `w . ~/test` will not work, as Bombadillo tries to write to downloadpath/~/test.

asdf added this to the 2.0.0 milestone 2019-11-04 02:23:45 +00:00

asdf self-assigned this 2019-11-04 02:23:45 +00:00

asdf added the

bug

label 2019-11-04 02:23:45 +00:00

asdf commented

2019-11-04 02:25:36 +00:00

I think this should wait for #73 to avoid merge conflicts.

sloum commented

2019-11-04 05:50:56 +00:00

This was by design. It seemed a simple model that mostly follows how people's web browsers are currently configured (though most browsers do also let you set it somewhere).

I am definitely up for allowing explicit save locations. Do we add another command? WRITETO/WT, or the like? Or do we try to tell if the argument being passed is a path vs a filename? Do we let people add on to the default path? If so, how do we differentiate that from a user just entering a path? Will we accept relative paths?

If we get to the bottom of those questions I think we will land in a good place and be able to code it up. I agree waiting until after #73 is a good idea.

This was by design. It seemed a simple model that mostly follows how people's web browsers are currently configured (though most browsers _do_ also let you set it somewhere). I am definitely up for allowing explicit save locations. Do we add another command? `WRITETO`/`WT`, or the like? Or do we try to tell if the argument being passed is a path vs a filename? Do we let people add on to the default path? If so, how do we differentiate that from a user just entering a path? Will we accept relative paths? If we get to the bottom of those questions I think we will land in a good place and be able to code it up. I agree waiting until after #73 is a good idea.

sloum commented

2019-11-05 01:29:22 +00:00

#73 has been merged in. Saving a file will no longer overwrite an existing file, instead adding a number to the end of the filename iteratively.

For this issue I think some decisions need to be made:

Are we interested in allowing alternate save paths?
If we are allowing alternate save paths do we try to do this as part of an existing command or make a new one?
We need to make sure we are handling "/", "~", etc. so that we dont get weird filenames attempting to be written.

#73 has been merged in. Saving a file will no longer overwrite an existing file, instead adding a number to the end of the filename iteratively. For this issue I think some decisions need to be made: - Are we interested in allowing alternate save paths? - If we are allowing alternate save paths do we try to do this as part of an existing command or make a new one? - We need to make sure we are handling "/", "~", etc. so that we dont get weird filenames attempting to be written.

asdf commented

2019-11-05 04:15:35 +00:00

I think this is an area where less functionality is better.

I would suggest going further, and remove the ability to specify a file name.

The majority of use cases could still be satisfied by simplified functionality. The client reports where a file has been downloaded, so the user can do what they like with it after that.

Does allowing a file name add much value? Yes, to some extent, but nothing that can't be done from the shell.

Furthermore, I would suggest that if we allow specifying a file name the convention seems to be that paths should also be supported. Looking at how other applications work:

vi allows :w a and :w /path/file and :w ~/file with tab completion
Firefox has a "save file as" type function, which opens a new screen where the save path and file name are prefilled but configurable. It has whatever gui shortcuts you might normally see.
Lynx has a download function that displays the URL and the suggested file name, plus a save to disk button. If you select save to disk, it asks you to enter a file name, but it's prefilled with the suggested name. It supports paths but does not support tab completion.

With all of that said, I would also be OK with allowing a file name to be specified but not allowing paths. It is the simpler approach.

I think this is an area where less functionality is better. I would suggest going further, and remove the ability to specify a file name. The majority of use cases could still be satisfied by simplified functionality. The client reports where a file has been downloaded, so the user can do what they like with it after that. Does allowing a file name add much value? Yes, to some extent, but nothing that can't be done from the shell. Furthermore, I would suggest that if we allow specifying a file name the convention seems to be that paths should also be supported. Looking at how other applications work: - vi allows `:w a` and `:w /path/file` and `:w ~/file` with tab completion - Firefox has a "save file as" type function, which opens a new screen where the save path and file name are prefilled but configurable. It has whatever gui shortcuts you might normally see. - Lynx has a download function that displays the URL and the suggested file name, plus a save to disk button. If you select save to disk, it asks you to enter a file name, but it's prefilled with the suggested name. It supports paths but does not support tab completion. With all of that said, I would also be OK with allowing a file name to be specified but not allowing paths. It is the simpler approach.

sloum commented

2019-11-07 05:56:09 +00:00

I can definitely see your point. It is a very simple update to remove the ability to do a save as.... I agree that since the path is shown to the user after the save (plus it is deterministic, since they set their save path) that it really isnt a needed feature.

Removing it will have the added benefit of cleaning up the manpage a little (it feels like a lot of :write commands all in a row).

I'll work on this tomorrow night and open a PR, unless you had already started?

I can definitely see your point. It is a very simple update to remove the ability to do a `save as...`. I agree that since the path is shown to the user after the save (plus it is deterministic, since they set their save path) that it really isnt a needed feature. Removing it will have the added benefit of cleaning up the manpage a little (it feels like a lot of `:write` commands all in a row). I'll work on this tomorrow night and open a PR, unless you had already started?

asdf commented

2019-11-07 11:58:44 +00:00

Glad we are agreed.

I had not yet started on this as I wanted to get your feedback first. If you feel like handling it then please go ahead!

Glad we are agreed. I had not yet started on this as I wanted to get your feedback first. If you feel like handling it then please go ahead!

sloum self-assigned this 2019-11-07 14:37:11 +00:00

asdf was unassigned by sloum

2019-11-07 14:37:13 +00:00

asdf commented

2019-11-08 05:31:37 +00:00

Fixed in #82. Thank you @sloum!

👍 1