Replace LinkedFiles backslashes with forward slashes#3364
Conversation
Ensures cross platform compatibility
add changelog
LinusDietz
left a comment
There was a problem hiding this comment.
The fix itself is fine, I just have one remark regarding a refactoring.
| * @param toCheck The String to check | ||
| * @return True if it starts with http://, https:// or contains www; false otherwise | ||
| */ | ||
| public static boolean isOnlineLink(String toCheck) { |
There was a problem hiding this comment.
URL validation is quite hard (potentially completely infeasible) in practice.
Is this implementation sufficient for our purposes? I mean "jabref.org" is a valid url, but would isOnlineLink() would return false.
My thoughts are to leave this method in the LinkedFile if it should serve as a "special purpose" link checker. If it should be a general method, in the util package, then the implementation would need to become more elaborate.
There was a problem hiding this comment.
I did not change the detection of urls, I just moved it
There was a problem hiding this comment.
that's what I mean: if you move it to an util package, it should be a general purpose method that works in 'all' cases.
If it stays in this class (potentially private) it's sufficient if it is good enough to be called in that specific context.
|
yes, agree, good point I readded it in the other PR |
Ensures cross platform compatibility
Fixes #3311
Tested this under windows and it works fine
gradle localizationUpdate?