-
Notifications
You must be signed in to change notification settings - Fork 399
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
[ConvertTo-ExcelXlsx] Accept a collection of paths #1605
Open
edwardmiller-mesirow
wants to merge
7
commits into
dfinke:master
Choose a base branch
from
edwardmiller-mesirow:fix-saveas
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[ConvertTo-ExcelXlsx] Accept a collection of paths #1605
edwardmiller-mesirow
wants to merge
7
commits into
dfinke:master
from
edwardmiller-mesirow:fix-saveas
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edwardmiller-mesirow
force-pushed
the
fix-saveas
branch
from
May 21, 2024 20:26
cdc9bf3
to
80b821b
Compare
edwardmiller-mesirow
force-pushed
the
fix-saveas
branch
2 times, most recently
from
May 21, 2024 21:22
5c2983e
to
e1e4cfd
Compare
edwardmiller-mesirow
changed the title
[ConvertTo-ExcelXlsx] Prevent error: "Unable to get the SaveAs property of the Workbook class"
[ConvertTo-ExcelXlsx] Accept a collection of paths
May 21, 2024
edwardmiller-mesirow
force-pushed
the
fix-saveas
branch
2 times, most recently
from
May 22, 2024 15:27
4d297bd
to
a8aeab9
Compare
edwardmiller-mesirow
force-pushed
the
fix-saveas
branch
from
May 22, 2024 15:32
a8aeab9
to
37cd733
Compare
@dfinke this is ready and seems to be considerably better in the case of converting an entire collection of XLS files. But should still work exactly as before in the case of just a single path. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If I loop through a big list of XLS files that I want to convert, it can throw an error at some point in the list:
This could be because of either file locking or issues performing round-trips to network locations. In a situation with network paths, it will make two full roundtrips per path entirely via Excel interop. And that could be a very error-prone situation. When converting the same list of files repeatedly, I was seeing it fail randomly each time on different files in that same list.
The changes in this PR:
This allows you to loop over a collection of paths that you want to convert, while only needing to create one COM object.
The $workbook object created by the Open method is used directly, instead of relying on the stateful ActiveWorkbook property of the $Excel object.
This optionally allows caching a copy of each file to the local Temp directory first, which can avoid both locking and network roundtrips. I made the CacheDirectory configurable as well, to handle the case where the default Temp directory isn't writeable. The default behavior is the same as before.
It also just uses ChangeExtension to set the extension since that will work a little better overall in cross-platform scenarios with different case sensitivity.