fix: filename too long in some cases #277
Conversation
| @@ -1,18 +1,18 @@ | |||
| package com.appland.appmap.record; | |||
|
|
|||
| import java.io.IOException; | |||
There was a problem hiding this comment.
It doesn't look like most of these changes to the imports are necessary. Can you limit them to what's required for the new functionality, please?
There was a problem hiding this comment.
Oh, sorry about that. Default IDE settings for new project and I missed it. Fixed in the new version
| public void testSanitizeFilename() { | ||
| String longFilename = "foo".repeat(100); | ||
| assertEquals("foo.appmap.json", Recorder.sanitizeFilename("foo")); | ||
| assertEquals( |
There was a problem hiding this comment.
It might be more obvious that this was operating as expected if the length of the return value from sanitizeFilename was checked.
Maybe something like
| assertEquals( | |
| Pattern p = Pattern.compile("([fo]*?)-538355d.appmap.json"); | |
| Matcher m = p.matcher(Recorder.sanitizeFilename(longFilename)); | |
| assertTrue(m.find()); | |
| assertEquals(255, m.group(0).length()); | |
| assertEquals(235, m.group(1).length()); |
There was a problem hiding this comment.
That makes sense, I'll use it like this. Fixed in the next version
buzzcz
force-pushed
the
fix/filename-too-long
branch
from
bf7bb2f to
70cc552
Compare
| @@ -1,19 +1,17 @@ | |||
| package com.appland.appmap.record; | |||
|
|
|||
| import static com.github.stefanbirkner.systemlambda.SystemLambda.tapSystemErr; | |||
There was a problem hiding this comment.
Sorry to be a nuisance about this, but looks like some more import change snuck in.
There was a problem hiding this comment.
It's ok, it's your job 😉 It's fixed
buzzcz
force-pushed
the
fix/filename-too-long
branch
from
70cc552 to
fc74f0a
Compare
apotterri
left a comment
apotterri
left a comment
There was a problem hiding this comment.
Looks good. Thanks again.
|
Perfect, thank you! |
|
🎉 This PR is included in version 1.26.8 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Thanks so much for your contribution! |
4 participants
This patch should similarly fix #75 as in getappmap/appmap-python#91