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

Support absolute paths for GetPictureCells and GetPictures functions #1988

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

slashdotdash
Copy link
Contributor

@slashdotdash slashdotdash commented Sep 4, 2024

PR Details

Support absolute paths for the GetPictureCells and GetPictures functions.

Description

Detects when a relationship target is an absolute path (starts with /) and trims the prefix. The behaviour for relative paths is unchanged.

Related Issue

Fixes #1987.

Motivation and Context

This PR solves an issue when attempting to get pictures from an Excel file which uses absolute paths in the XML for the relationship target, such as in the example XML snippet below.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships">
    <Relationship Id="rId1"
        Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/image"
        Target="/xl/media/image1.jpg" />
</Relationships>

How Has This Been Tested

The changes have been tested locally using the problematic Excel file attached to the issue and the PR extends the existing TestGetPicture test (in picture_test.go) to get picture cells and pictures with an absolute target path in the drawing relationship.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 4, 2024
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.31%. Comparing base (0447cb2) to head (df20658).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1988   +/-   ##
=======================================
  Coverage   99.31%   99.31%           
=======================================
  Files          32       32           
  Lines       25267    25277   +10     
=======================================
+ Hits        25095    25105   +10     
  Misses         92       92           
  Partials       80       80           
Flag Coverage Δ
unittests 99.31% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your contribution.

@xuri xuri merged commit aca04ec into qax-os:master Sep 4, 2024
37 checks passed
@slashdotdash slashdotdash deleted the get-picture-absolute-path branch September 4, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Status: Bugfix
Development

Successfully merging this pull request may close these issues.

GetPictureCells and GetPictures functions don't support images with absolute path targets
2 participants