search

nybbles / obsidian-pocket

Pocket integration for Obsidian
MIT License
234 stars 22 forks source link

Limit filename length due to Windows filesystem #101

Closed GollyTicker closed 2 years ago

GollyTicker commented 2 years ago

I had issues with long filenames generated by the obsidian-pocket plugin, when the original Title/URL from Pocket was too long. This can happen for certain Reddit posts with long titles.

I added a small fix which truncates filenames to the length 255 - 15. I have manually edited the change into my local main.js and tested, that it works as expected.

I think it is reasonable to expect, that the entire title does not need to be in the filename - especially when it's so long. Alternatively, we could add an option to toggle this sanitization, if necessary.

What do you think? Could we merge this? @nybbles

GollyTicker commented 2 years ago

@nybbles Could we merge this?

nybbles commented 2 years ago

@GollyTicker apologies for the long response time. Did you test what happens when there's already a file with the shortened name?

e.g. currently if there's already a somefile and we try to save another somefile, it will be saved as somefile 2 instead.

The relevant logic that adds a version number is here: https://github.com/nybbles/obsidian-pocket/blob/d059632c337399b93391219faf8cc3ac9c1882ec/src/ItemNote.ts#L244-L256.

The number of duplicates goes up to 1000, so with the space separator that's five extra characters. I think the simplest solution would be to just do 255 - 5.

Actually hold on, I see that you're doing 255 - 15 already - where does 15 come from?

Oh yes and thank you for putting up a pull request for the issue you raised. It's much appreciated.

GollyTicker commented 2 years ago

I didn't test specifically for this scenario, but because we still leave enough characters (255-15) for line ending and the de-duplication number, I think this should be fine.

I'd did test the logic with smaller and larger titles.

nybbles commented 2 years ago

Where does the value for 15 come from? Do you think it would be reasonable to set it to 5?

nybbles commented 2 years ago

This truncation should also only be happening on Windows, since that's the only platform that has this requirement on filenames.

nybbles commented 2 years ago

Actually this happens on Mac too

nybbles commented 2 years ago

looks like 255 characters for filenames is a pretty common limit across various filesystems, so I think it's okay to apply this limit to all supported Obsidian platforms.

GollyTicker commented 2 years ago

I've updated the PR @nybbles

nybbles commented 2 years ago

Thanks for your fix. I'll cut a release later today.

nybbles commented 2 years ago

FYI this fix is released and installable now. Sorry about the delay.