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

[Feature] "Show PDF" button #72

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

dvdvgt
Copy link
Contributor

@dvdvgt dvdvgt commented Apr 5, 2023

This PR is in response to #69.

Discussion

I am not really happy how the file extension/type is currently manually changed from .typ to .pdf. I'll happily implement something more robust if somebody has a recommendation.

@nvarner nvarner force-pushed the feature/show-preview-button branch from 81aea5a to 65ae7a2 Compare April 13, 2023 00:47
@nvarner
Copy link
Owner

nvarner commented Apr 13, 2023

At present, I think the filename conversion is okay. Doing better depends on finding a good way to decide which files get compiled into PDFs and what those PDFs should be called, which hasn't happened yet.

@nvarner
Copy link
Owner

nvarner commented Apr 13, 2023

@dvdvgt can you fix the failing lint? This will be ready to merge after that.

@dvdvgt
Copy link
Contributor Author

dvdvgt commented Apr 13, 2023

Should be good now. Feel free to squash all commits into one.

@nvarner
Copy link
Owner

nvarner commented Apr 13, 2023

Oops, that latest failure might have been my fault; I needed an extra feature in a dependency, and I force pushed while preparing a PR to upstream the changes. My changes were upstreamed but not yet released, so it's okay to keep the elsa Git dependency. To fix the changed Git commit hash, can you try cargo update elsa? I think that will resolve it.

@nvarner nvarner merged commit 39dd25e into nvarner:master Apr 13, 2023
3 checks passed
@nvarner
Copy link
Owner

nvarner commented Apr 13, 2023

Thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants