-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@location tag #3011
Merged
Merged
@location tag #3011
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
f4706ca
Adding Tag.location
OAGr 1b67b11
Merge branch 'Forwardref-fix' into with-location
OAGr 2283968
Addes @location tag
OAGr 21e55c3
Added a new class, ValueTagContainer, to help with displaying tags
OAGr 7acc488
Adds comment and changeset
OAGr a14d13a
Quick fix
OAGr c575233
Renaming
OAGr 5c57057
Added test, removed dead code
OAGr 4dc9783
Addressed CR comments
OAGr e1ee728
Fixed tests
OAGr File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
"@quri/squiggle-lang": patch | ||
"@quri/squiggle-components": patch | ||
--- | ||
|
||
Adds new @location tag and Tag.getLocation |
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -303,6 +303,7 @@ fn = {|e| e} | |
bar = [x, fn] | ||
|
||
@startOpen | ||
@location | ||
s = 4 to 10 | ||
|
||
@startClosed | ||
|
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { Location, LocationRange } from "peggy"; | ||
|
||
import { ImmutableMap } from "../utility/immutableMap.js"; | ||
import { Value, vNumber, vString } from "./index.js"; | ||
import { ValueTags } from "./valueTags.js"; | ||
import { vDict, type VDict } from "./VDict.js"; | ||
|
||
function locationRangeToValue(locationRange: LocationRange) { | ||
function locationToDict(location: Location): VDict { | ||
return vDict( | ||
ImmutableMap([ | ||
["line", vNumber(location.line)], | ||
["column", vNumber(location.column)], | ||
["offset", vNumber(location.offset)], | ||
]) | ||
); | ||
} | ||
let items: [string, Value][] = [ | ||
["start", locationToDict(locationRange.start)], | ||
["end", locationToDict(locationRange.end)], | ||
]; | ||
if (typeof locationRange.source === "string") { | ||
items = [["source", vString(locationRange.source)], ...items]; | ||
} | ||
return vDict(ImmutableMap(items)); | ||
} | ||
|
||
// This class is meant for functions that depend on Value. BaseValue depends on ValueTags, so we can't call many functions there. Instead, we call them here. | ||
export class ValueTagsWrapper { | ||
constructor(public value: ValueTags) {} | ||
|
||
toList(): [string, Value][] { | ||
const result: [string, Value][] = []; | ||
const { value } = this.value; | ||
if (value.name?.value !== undefined) { | ||
result.push(["name", value.name]); | ||
} | ||
if (value.doc?.value !== undefined) { | ||
result.push(["doc", value.doc]); | ||
} | ||
if (value.showAs !== undefined) { | ||
result.push(["showAs", value.showAs]); | ||
} | ||
if (value.numberFormat !== undefined) { | ||
result.push(["numberFormat", value.numberFormat]); | ||
} | ||
if (value.dateFormat !== undefined) { | ||
result.push(["dateFormat", value.dateFormat]); | ||
} | ||
if (value.hidden !== undefined) { | ||
result.push(["hidden", value.hidden]); | ||
} | ||
if (value.notebook !== undefined) { | ||
result.push(["notebook", value.notebook]); | ||
} | ||
const _exportData = this.value.exportData(); | ||
if (_exportData !== undefined) { | ||
result.push(["exportData", _exportData]); | ||
} | ||
|
||
if (value.startOpenState !== undefined) { | ||
result.push(["startOpenState", value.startOpenState]); | ||
} | ||
if (value.location !== undefined) { | ||
result.push(["location", locationRangeToValue(value.location)]); | ||
} | ||
|
||
return result; | ||
} | ||
|
||
toMap(): ImmutableMap<string, Value> { | ||
return ImmutableMap(this.toList()); | ||
} | ||
|
||
toString(): string { | ||
return this.toList() | ||
.map(([key, value]) => `${key}: ${value.toString()}`) | ||
.join(", "); | ||
} | ||
|
||
location(): VDict | undefined { | ||
const _location = this.value.location(); | ||
return _location ? locationRangeToValue(_location) : undefined; | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
import { ImmutableMap } from "../utility/immutableMap.js"; | ||
import { LocationRange } from "peggy"; | ||
|
||
import { Err, fmap, mergeMany, Ok, result } from "../utility/result.js"; | ||
import { Value } from "./index.js"; | ||
import { type VBool } from "./VBool.js"; | ||
import { type VDict } from "./VDict.js"; | ||
import { VDict } from "./VDict.js"; | ||
import { type VString } from "./VString.js"; | ||
|
||
// Note: this file can't call any `vType` constructors; it would cause a circular dependency because of `BaseValue` -> `ValueTags`. | ||
|
@@ -18,6 +19,7 @@ export type ValueTagsType = { | |
notebook?: VBool; // can be set on arrays | ||
exportData?: VDict; // should be { sourceId: String, path: List(String) } | ||
startOpenState?: VString; | ||
location?: LocationRange; | ||
}; | ||
|
||
type ValueTagsTypeName = keyof ValueTagsType; | ||
|
@@ -32,6 +34,7 @@ const valueTagsTypeNames: ValueTagsTypeName[] = [ | |
"notebook", | ||
"exportData", | ||
"startOpenState", | ||
"location", | ||
]; | ||
|
||
function convertToValueTagsTypeName( | ||
|
@@ -54,43 +57,25 @@ function convertToValueTagsTypeName( | |
export class ValueTags { | ||
constructor(public value: ValueTagsType) {} | ||
|
||
toList(): [string, Value][] { | ||
const result: [string, Value][] = []; | ||
const { value } = this; | ||
if (value.name?.value) { | ||
result.push(["name", value.name]); | ||
} | ||
if (value.doc?.value) { | ||
result.push(["doc", value.doc]); | ||
} | ||
if (value.showAs) { | ||
result.push(["showAs", value.showAs]); | ||
} | ||
if (value.numberFormat) { | ||
result.push(["numberFormat", value.numberFormat]); | ||
} | ||
if (value.dateFormat) { | ||
result.push(["dateFormat", value.dateFormat]); | ||
} | ||
if (value.hidden) { | ||
result.push(["hidden", value.hidden]); | ||
} | ||
if (value.notebook) { | ||
result.push(["notebook", value.notebook]); | ||
} | ||
const _exportData = this.exportData(); | ||
if (_exportData) { | ||
result.push(["exportData", _exportData]); | ||
} | ||
|
||
if (value.startOpenState) { | ||
result.push(["startOpenState", value.startOpenState]); | ||
} | ||
return result; | ||
isEmpty() { | ||
return ( | ||
this.value.name === undefined && | ||
this.value.doc === undefined && | ||
this.value.showAs === undefined && | ||
this.value.numberFormat === undefined && | ||
this.value.dateFormat === undefined && | ||
this.value.hidden === undefined && | ||
this.value.notebook === undefined && | ||
this.value.exportData === undefined && | ||
this.value.startOpenState === undefined && | ||
this.value.location === undefined | ||
); | ||
} | ||
|
||
isEmpty() { | ||
return this.toList().length === 0; | ||
toString(): string { | ||
return Object.entries(this.value) | ||
.map(([key, value]) => `${key}: ${value?.toString()}`) | ||
.join(", "); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could also write this out. The previous setup would also work, though it's nice to organize it in |
||
|
||
omit(keys: ValueTagsTypeName[]) { | ||
|
@@ -105,16 +90,6 @@ export class ValueTags { | |
return fmap(params, (args) => this.omit(args)); | ||
} | ||
|
||
toMap(): ImmutableMap<string, Value> { | ||
return ImmutableMap(this.toList()); | ||
} | ||
|
||
toString(): string { | ||
return this.toList() | ||
.map(([key, value]) => `${key}: ${value.toString()}`) | ||
.join(", "); | ||
} | ||
|
||
merge(other: ValueTagsType) { | ||
return new ValueTags({ | ||
...this.value, | ||
|
@@ -150,6 +125,10 @@ export class ValueTags { | |
return this.value.notebook?.value; | ||
} | ||
|
||
location() { | ||
return this.value.location; | ||
} | ||
|
||
startOpenState(): "open" | "closed" | undefined { | ||
const { value } = this.value.startOpenState ?? {}; | ||
if (!value) { | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could safely skip OOP here and use a module-as-plain-object:
And then
return tagUtils.toValueMap(tags)
.(AFAICT the only function you need here is
toValueMap
, but exporting many small functions one by one can be annoying and less readable, so a top-level object is more convenient)