Skip to content

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
@codecov
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
@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

No open 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