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

Guess the output format based on the --output argument #160

Closed
Deuchnord opened this issue Apr 4, 2021 · 11 comments
Closed

Guess the output format based on the --output argument #160

Deuchnord opened this issue Apr 4, 2021 · 11 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@Deuchnord
Copy link
Member

(Feature request by @AmauryCarrade on Discord)

Is your feature request related to a problem? Please describe.
Currently, to export in a PDF file, we need to give two arguments to the command:

  • --format=pdf
  • --output=myfile.pdf
    Giving both arguments, even though acceptable, is redundant.

Describe the solution you'd like

  • The --format argument becomes optional.
  • If both --format and --output is given, then the behavior stays the same as today: the content is output in the given file using the given format, even though the extension does not match the format.
  • If only --output is given, then guess the format to use from it:
    • --output=myfile.txt ou --output=myfile (without extension) will output a file containing the text usually returned in standard output
    • --output=myfile.json will output a file containing a JSON content
    • --output=myfile.pdf will output a file containing the PDF content
  • If only --output is given, but the file format is not recognized, then an error is returned and nothing is done.
@Deuchnord Deuchnord added enhancement New feature or request good first issue Good for newcomers labels Apr 4, 2021
@Deuchnord Deuchnord added this to the 0.11 milestone Apr 4, 2021
@Deuchnord Deuchnord added this to To do in Version 1.0.0 via automation Apr 4, 2021
@Deuchnord Deuchnord changed the title Guess the format based on the output file Guess the output format based on the output file May 3, 2021
@github-actions
Copy link

github-actions bot commented Jun 2, 2021

This issue has not been active since a lot of time. If you think it is still valid, feel free to reply with a simple comment. Without any update, I will close it in 7 days.

@github-actions github-actions bot added the Stale label Jun 2, 2021
@Deuchnord
Copy link
Member Author

Nope, you don't want to close that. 👀

@Deuchnord Deuchnord removed the Stale label Jun 4, 2021
@github-actions
Copy link

github-actions bot commented Jul 4, 2021

This issue has not been active since a lot of time. If you think it is still valid, feel free to reply with a simple comment. Without any update, I will close it in 7 days.

@github-actions github-actions bot added the Stale label Jul 4, 2021
@Deuchnord Deuchnord removed the Stale label Jul 5, 2021
@Deuchnord Deuchnord changed the title Guess the output format based on the output file Guess the output format based on the --output argument Jul 14, 2021
@Deuchnord Deuchnord self-assigned this Jul 22, 2021
@nicfb
Copy link
Contributor

nicfb commented Jul 28, 2021

I see that you self-assigned this issue. Are you currently working on this? If not, I would love to give it a shot.

@Deuchnord
Copy link
Member Author

Hello, no, I haven't started it yet, you can take it.
Thank you! 😄

@Deuchnord Deuchnord removed their assignment Jul 29, 2021
@nicfb
Copy link
Contributor

nicfb commented Aug 6, 2021

I am having some issues running using the different output types, specifically on this line. When I use json or text, I have to remove the "b" from the arguments so the file isn't opened in binary mode or else I see this error: TypeError: a bytes-like object is required, not 'str'. I have to add it back to use the pdf output format or else I see the same error. Am I doing something wrong?

@Deuchnord
Copy link
Member Author

No, I think I see what you are facing. It is a bit tricky, since the PDF format is a binary format, but not the plain text and JOSN format.
I think the most elegant way to fix this would be to create a small function you would call in the second argument of open() and that would return the correct mode depending on the output format.
Something like this:

def get_opening_mode(format: str) -> str:
    if format == "pdf":
        return "wb"

    return "w"

WDYT?

@nicfb
Copy link
Contributor

nicfb commented Aug 6, 2021

Ok, just wanted to make sure. I like the solution you suggested!

@nicfb
Copy link
Contributor

nicfb commented Aug 11, 2021

I have something working now. I think all that is left is to add some tests. Would I add these in the end to end test script?

@Deuchnord
Copy link
Member Author

I think you can add them just after the existing export ones :)

@Deuchnord
Copy link
Member Author

Btw @nicfb, be careful, there is a license change, Kosmorro is rolling back to GNU AGPL instead of CeCILL (#196).
Your PR can stay under CeCILL if you want (AFAIK it is AGPL-compatible), just making sure you are aware about that :)

@Deuchnord Deuchnord moved this from To do to In progress in Version 1.0.0 Aug 18, 2021
Version 1.0.0 automation moved this from In progress to Done Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Version 1.0.0
  
Done
Development

No branches or pull requests

2 participants