v0.19 format reader and generic deserializer #1435
No reviewers
Labels
No labels
UX
active development
backlog
blocker
bootstrap
bounty
bug
dependencies
discussion
documentation
duplicate
enhancement
flaky test
help wanted
invalid
javascript
question
release
tendentious
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
mighty-gerbils/gerbil!1435
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "v0.19-serde-deserialize"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implement a reader that can read format output
format readerto v0.19 format reader and generic deserializerJust a few elements to get you going while I review the rest...
@ -233,3 +231,1 @@(printable (and (not all-slots-printable?) (make-props! print:)))(all-slots-equalable? (or transparent? (eq? #t (agetq equal: properties))))(equalable (and (not all-slots-equalable?) (make-props! equal:)))(def (no-alist? key alist)Not a good function name. Suggestions:
all-slots?
all-slots-covered?
Or something like that.
Then a comment that says that unless there's an alist override, all slots are covered indeed.
yeah, you right. renamed to has-no-alist-overide?
I much prefer naming the function after the ultimate intention than the implementation or representation detail. The implementation detail is obvious already by reading the source.
sure, but this is an intentional name!
Nah. First it's override, not overide, and I prefer a positive meaning
all-slots-covered?or something is better.ok, let me find a better name.
sure, but this is an intentional name!its not though, its some slots. really a programmer override.
i'll fix the typo.
fixed
@ -236,0 +234,4 @@(else #t)))(let* ((transparent?(agetq transparent: propertiesIs the indentation funky only in the review window or do you need to fix it in the file?
it was me, fixed.
@ -316,0 +318,4 @@(cons klass (cdr subclasses)))))(else(set! (&class-type-properties super)(cons [subclasses: klass] props)))))))In general, I much prefer slots over properties. Ideally, properties as alist should only exist as syntax, and maybe as a mechanism for future-handled things. But not for regular properties used by the system.
i think its appropriate here. changing slots is a more heavy operation, lets reconsider in the future.
Then let's open an issue to do these kinds of things properly.
You know what? If we do things properly, I can proudly document the Gerbil Way in the MOP chapter of my book.
upon in person discussion, lets follow up on this.
we also need a class lock, so it becomes a bootstrap pain i would like to avoid in this pr
#1437
@ -434,3 +450,3 @@(slot-flags (vector-ref fields (fx+ i 1)))(next-i (fx+ i 3)))(if (fx= (fxand slot-flags 1) 0) ;; printable flag(if (fx= (fxand slot-flags 1) 0) ;; printabke flagWTF typo. You might want to install a typo-detector in your emacs config.
lol yes. fixing.
fixed
@ -440,3 +456,3 @@(loop next-i(fx+ offset 1)(cons (cons slot-name offset) r))))r)))Yikes, what was that bug?
yes.
@ -289,0 +299,4 @@(qsyntax :- :symbol) ; macro symbol for #` or #f(unsyntax :- :symbol) ; macro symbol for #, or #f(sunsyntax :- :symbol) ; macro symbol for #,@ or #f)Let's give the full names quasiquote, unquote-splicing, quasisyntax, unsyntax-splicing. It doesn't make sense to have some names clash with the official names and other not but use ad hoc abbreviations.
ok
fixed
@ -8,3 +10,4 @@./reader)(export #t)(def (format-write (writer : BufferedWriter) obj (env : WriteEnv))Maybe rename to write-formatted, read-formatted?
eh, it is somewhat inconsistent.
basically its format, the subsystem, and its variants of read/write.
@ -0,0 +16,4 @@(def reader-test(test-suite "read format written objects"(test-case "read integers"(check (read-object-from-string "1234" (reader-environment))Should (reader-environment) be an optional argument?
(With a test that it indeed behaves as such in the four cases that the parameter is overridden or not, vs specified explicitly or not.)
probably, i'll change the signature but keep explicit for the tests.
done
@ -0,0 +162,4 @@))(def reader-dag-test(test-suite "read dag format written objects"Define a function for cases that work for the tree case, run them with dag and cycles too?
Define a function for cases that work for the dag case, run them with cycles too?
thats something we can ask the LLM to do. lets create issue.
#1438
@ -0,0 +23,4 @@=> ReaderEnv(ReaderEnvrenv: (ReadEnv allow-class: allow-class?allow-procedure: allow-procedure?I don't understand ReaderEnv vs ReadEnv. And the name "renv" isn't great—maybe "read-env"? "opt" should also probably be "options".
lets the serde envs Context.
opt we use everywhere...
Well, then we need an issue to have longer names everywhere.
renamed serde envs to contexts
i am keeping opts, i call options opts everywhere and it doesnt cause confusion.
@ -0,0 +31,4 @@special: __special-readtable))(def (parse (reader : BufferedReader)(env : ReaderEnv)indentation issue?
yes
@ -0,0 +93,4 @@(def (parse-special (reader : BufferedReader)(env : ReaderEnv)(anchor : Anchor))reindent?
done
@ -0,0 +335,4 @@(:- (##vector-ref __symbolic-delimiter-chars int):boolean))))(def (parse-special-index (reader : BufferedReader)I don't like the word "special" here. If it's for the sharp reader, maybe "sharp" instead?
ok, will rename.
renamed
fixed some indentation issues
Minor typos, otherwise LGTM
@ -47,3 +50,3 @@(def (fold-format-ops-for-stx stx writer env ops)(def (fold-format-ops-for-stx stx writer ctx ops)(with-identifiers ((writer.write-char writer writer ".write-char-utf8")Align?
@ -1,4 +1,4 @@;;; -*- Gerbil -*-;;; -*- Gerbilo -*-Is Gerbilo the miniKanren version of Gerbil?
no its me struggling with my new toy spaceship control keyboard
@ -15,13 +15,13 @@(defstruct FormaterProper spelling is Formatter
ugh, fixing
@ -0,0 +10,4 @@./api./reader./io./reader-test-support)Uh, align?
it is, no?
oh, readers together