jhthorsen / app-mojopaste

Pastebin
38 stars 19 forks source link

Support for file upload #24

Closed afresh1 closed 1 year ago

afresh1 commented 2 years ago

We added this back in 0.15 and then got distracted and didn't create a PR for you. Have now ported those changes forward and gotten the PR out.

This should close #20

jhthorsen commented 2 years ago

Thanks for the PR, but it's very hard to evaluate because it combines a lot of different changes:

  1. Image (non-text) upload (Not sure if I want to support this)
  2. Change from a simple file to a meta and content file
  3. Looks like it breaks _save_p() from other custom backends
  4. Maybe more things I missed...

The title of this PR is also confusing, since mojopaste already supports uploads...

afresh1 commented 2 years ago

I've been trying to figure out how to move forward on this, but need some additional guidance. The biggest is that you say it already supports file uploads, but I am not quite sure I follow how, could you expand on that?

If I tidy up the first commit and remove the other two, would that be better for review and more likely to be accepted? It seems that less than 0.15% of our internal pastes are non-text, so I think we would be OK with losing that feature.

jhthorsen commented 1 year ago

Support for uploads

I've totally missed out on what you meant here. You're absolutely right that Mojopaste does not support file uploads. I think the way I would solve this is to add drag/drop support with JavaScript. The dropped file would then be displayed in the text area after dropped. The help text has too be adjusted as well, or maybe add a background image.

Download filename

I'll take 2d93561b239bdbdb9ce446e44bb8e1d695cd563f if you open a PR for just that.

Non-text uploads

I'm still unsure about this one. I might change my mind though...