Fix URL #4
Fix URL #4
Conversation
|
From what I can tell, the old URLs aren't migrated to the new format. Is this intended? If so, I better quickly backup my links so I can fix them when they break. |
|
Good point. @Orpheus-3145 please add logic to support the old link format, while never generating new ones in that style |
Deploying javafiddle with
|
| Latest commit: |
47e89ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e1f75204.javafiddle.pages.dev |
| Branch Preview URL: | https://fixurl.javafiddle.pages.dev |
|
support for legacy URLs added |
saarikoski-jules
left a comment
saarikoski-jules
left a comment
There was a problem hiding this comment.
I've left a bunch of cosmetic and review ease related comments. Please address those, do some general cleanup and structure your commits in a way where each commit contains minimal updates on the same change and is it's own logical whole. If you're also copying code for the original url handling, you should just originally keep that functionality in the code base, rather than deleting it and then adding it back.
src/lib/Repl.svelte
Outdated
| updated: $fiddleUpdated, | ||
| files: $files | ||
| }); | ||
There was a problem hiding this comment.
Avoid unnecessary changes like these
src/routes/+layout.ts
Outdated
| @@ -0,0 +1,32 @@ | |||
| import { decompress, type Fiddle } from '$lib/compress-fiddle.js'; | |||
There was a problem hiding this comment.
It would simplify review and improve history if you made your changes to this file in one commit and renamed it in another one. Please also do the same for other applicable cases
| import { favouriteIndex, favourites, fiddleTitle, fiddleUpdated, files } from '../state'; | ||
| import { onNavigate } from '$app/navigation'; | ||
There was a problem hiding this comment.
Why is this line changed? Is there some spurious white space character here?
src/routes/+layout.ts
Outdated
| @@ -1,32 +1,17 @@ | |||
| import { decompress, type Fiddle } from '$lib/compress-fiddle.js'; | |||
| // import { decompress, type Fiddle, defaultFiddle } from '$lib/compress-fiddle.js'; | |||
There was a problem hiding this comment.
Don't leave around commented out code
| }); | ||
| $favouriteIndex = index; | ||
| // update URL with fragment | ||
| const fiddleFragmentURL = compress($favourites[index]); |
There was a problem hiding this comment.
I don't see why the order of these lines has changed, it shouln't make a logical difference if I'm not wrong
saarikoski-jules
left a comment
saarikoski-jules
left a comment
There was a problem hiding this comment.
Looks mostly good to me, a few small things. While you're at it, would you remove the console.log that occurs on every key input (you can do it in a new commit since its unrelated)
saarikoski-jules
left a comment
saarikoski-jules
left a comment
There was a problem hiding this comment.
@alexp-sssup This pr is ready for your final review
The fiddle data (code files, title, ...) previously encoded and stored inside a URL parameter, is now stored in a fragment; main changes: