Choose Context modal doesn't show bad non-context strings #516
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, if your first line wasn't a use context declaration, the Choose Context modal would just grab an arbitrary substring of your first line, resulting in gibberish. This change first checks if the first line is a use context. If so, it shows the current context. If not, it shows the current context as blank and lets you enter a new one.
blerner
reviewed
Apr 18, 2024
Member
blerner
left a comment
blerner
left a comment
There was a problem hiding this comment.
LGTM, just some minor refactoring to make it less repetitive with existing code.
| $("#download").append(downloadElt); | ||
| }); | ||
|
|
||
| const CONTEXT_PREFIX = "use context "; |
Member
There was a problem hiding this comment.
This should be /^use context\s+/, so that the .slice on line 498 trims any interstitial whitespace between the context and the actual context value.
Actually, could you reuse firstLineIsNamespace from beforePyret, and expose it along with setContextLine, at
code.pyret.org/src/web/js/beforePyret.js
Line 294 in ef0d4da
CPO.editor.firstLineIsNamespace? You'd fix up the regex on line 225, (it shouldn't end in .*, but rather \s+. Then maybe refactor lines 495-498 of your code below into a function getContextFromFirstLine() that is likewise made available?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #399
Previously, if your first line wasn't a
use contextdeclaration, the Choose Context modal would just grab an arbitrary substring of your first line, resulting in gibberish.This change first checks if the first line is actually a
use context. If so, it shows the current context. If not, it shows the current context as blank and lets you enter a new one.