Skip to content

Add logic to skip hidden files when including non-FSH IG resources#580

Merged
ngfreiter merged 5 commits intoFHIR:masterfrom
masnick:non-fsh-ig-resource-hidden-file-fix
Aug 28, 2020
Merged

Add logic to skip hidden files when including non-FSH IG resources#580
ngfreiter merged 5 commits intoFHIR:masterfrom
masnick:non-fsh-ig-resource-hidden-file-fix

Conversation

@masnick
Copy link
Contributor

@masnick masnick commented Aug 24, 2020

This prevents an error when macOS silently adds a .DS_Store file to one of the folders in ig-data/input/{supported-resource-input-directory}/.

I ran into this when trying to include some .json format instances in an IG under ig-data/input/examples/. When modifying the .json files in the examples/ folder, macOS will sometimes silently add a .DS_Store file into examples/.

When this happens, the sushi command will fail with a generic "Invalid file detected in directory /path/to/examples/" message. Because the .DS_Store file is hidden by the OS in Finder, this issue is hard to debug.

This change will ignore any files that begin with . to avoid this problem.

This prevents an error when macOS silently adds a `.DS_Store` file
to one of the folders in `ig-data/input/{supported-resource-input-directory}/`.
Copy link
Contributor

@ngfreiter ngfreiter left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! There is one small change that would be good before merging, but all looks good besides that.

let resourceJSON: any;
try {
if (file.endsWith('.json')) {
if (file.startsWith('.')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have come across this situation in a different part of our code, and there we used a package called junk to filter out hidden files like .DS_Store but also certain Windows hidden files like Thumbs.db. I think it would be best to be consistent and use that here as well. To use junk, you'd want to add:

import junk from 'junk';

to the top of load.ts, and then line 197 would be something like:

if (junk.is(file)) {

The junk.is function just tests if the filename is on junks list of system files.

@cmoesel
Copy link
Member

cmoesel commented Aug 28, 2020

@masnick -- we're hoping to do a release of SUSHI today. If you can fix up this PR today, we can get it in the release...

@cmoesel
Copy link
Member

cmoesel commented Aug 28, 2020

If you can't we'll just hold it for the next release then.

@masnick
Copy link
Contributor Author

masnick commented Aug 28, 2020

@ngfreiter thanks for the review, I've done as you suggested.

@cmoesel assuming the automated checks pass this should be ready to go into the release. I'm stepping away for a few hours now so I won't be able to fix problems immediately, so if this gets bumped to the next release because of that that's ok with me :)

@masnick masnick requested a review from ngfreiter August 28, 2020 13:56
@masnick
Copy link
Contributor Author

masnick commented Aug 28, 2020

Ok, the checks passed and I tested this locally to ensure it works as intended. I think it's ready to merge in unless you see other issues. Thanks!

@ngfreiter ngfreiter merged commit 26f4cfe into FHIR:master Aug 28, 2020
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.

3 participants