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

Track the full paths of attachments in the Attachment object #71

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

tagatac
Copy link
Owner

@tagatac tagatac commented Nov 17, 2024

What is changing:

  1. Disambiguate Filename and Filepath in the Attachment object.
  2. Replace relative export paths with absolute ones.

Why this change is being made:

  1. Previously, we were computing the absolute path of the attachments in multiple places in different ways depending on the options given and whether the attachment was converted or not. This was confusing and error prone (see Fix/review Exit with code 1 due to network error: ContentNotFoundError - seems to be a PNG file type issue..? #70).
  2. As a result of the first change, we are using the copied path as the file to embed or reference. When embedding in PDFs, relative paths do not work, so we need to specify the copied path (under the export path in the case of the --copy-attachments option) as an absolute path.

Related issue(s): Fixes #70

Follow-up changes needed: None AFAIK

Is the change completely covered by unit tests? If not, why not?: No, similarly to converting the attachments path to an absolute path, we are not testing failures of the filepath.Abs function.

@tagatac tagatac merged commit 3c6af3e into main Nov 17, 2024
3 of 4 checks passed
@tagatac tagatac deleted the copy-pdf branch November 17, 2024 15:16
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.

Fix/review Exit with code 1 due to network error: ContentNotFoundError - seems to be a PNG file type issue..?
1 participant