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

Allow using any text file, not just .txt ones #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hnOsmium0001
Copy link

Close #1

@Andre601
Copy link
Contributor

I'm actually tempted to fork this repo, to make my own (actually updated) version as this one seems somewhat outdated and no longer maintained at all.

@Andre601
Copy link
Contributor

Looking at this, couldn't this cause conflicts? In a JSON file f.e. do you have a strict format, which may not be of good use for this site here.
The main point I get from this site is to render raw content as if it was an actual Discord message, so having to render JSON here seems pointless as you could also just download and open the file yourself to see the content of it.

@jagrosh
Copy link
Member

jagrosh commented Jan 28, 2021

There are several concerns with this, which is why I haven't merged it.

  • Allowing arbitrary files could lead to attempting to load non-plaintext (such as binary) files, which could have unintended or poor results, especially if these files are large.
  • The Discord client already automatically converts messages longer than 2000 characters to .txt if you have upload permissions. This doesn't detract from supporting more file types, but just makes them less necessary.
  • If the parameter syntax would change, I would prioritizing switching to url fragment syntax (#), as sending the query parameter (?txt=) to the webpage itself isn't necessary (since all data loading is done locally).

@hnOsmium0001
Copy link
Author

Looking at this, couldn't this cause conflicts? In a JSON file f.e. do you have a strict format, which may not be of good use for this site here.

The intention of this PR was so that things log debug.log that people directly upload, no parsing are done--all files are treated as if they are text files.

Allowing arbitrary files could lead to attempting to load non-plaintext (such as binary) files, which could have unintended or poor results, especially if these files are large.
Indeed a problem, and simply saying the user know what they are doing is a bit unideal.

File size should be doable (have to do some research). However I don't know if there is a way to detect binary vs text file in the client, and they could rename a binary file as message.txt anyways.

If the parameter syntax would change, I would prioritizing switching to url fragment syntax (#), as sending the query parameter (?txt=) to the webpage itself isn't necessary (since all data loading is done locally).

Sure, do you want to me to do it in the same PR (asked because some people hate multi-purpose PRs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow opening arbitary text foramt files, not just .txt ones
3 participants