Improve onboarding #514
Improve onboarding #514
Conversation
Some of the .asm word list comments were out of date anyway, so I moved them into durexforth.asm, making it a single searchable index for the whole dictionary.
|
Actually please don't merge yet. f449cf9 is still a bit hairy to my liking. I've never submitted a pull request before, I assume it'll track my master if I rebase and force push? It'll be a couple hours before I get home though. |
|
Hi!
Thanks for considering to make a contribution!
Take your time, and I can review it once you feel ready.
I do not know how rebase and force push will work out. Usually, I just make
plain commits to a branch, and merge it once it is done. I would guess
either way is fine.
fre 3 feb. 2023 kl. 19:20 skrev ekipan ***@***.***>:
… Actually please don't merge yet. f449cf9
<f449cf9>
is still a bit hairy to my liking.
I've never submitted a pull request before, I assume it'll track my master
if I rebase and force push? It'll be a couple hours before I get home
though.
—
Reply to this email directly, view it on GitHub
<#514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAY34OYAFZOMVYQ4SQMU2S3WVVD7JANCNFSM6AAAAAAUQOGT4U>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Saves a counter juggle.
Forgot to apply PROGRAM_BASE earlier as well.
|
I'm happy with the log. Please review. |
|
For some reason, the build does not pass with these changes. Any idea what might cause it? |
|
Looks like it ran out of cycles? I'm not sure why the rebased HEAD would if the old one didn't. |
|
I put the base.fs changes on a branch and rewound master, pardon me while I use your build system for my own testing. If this is unsuccessful I'll retract the PR for now until I get my own environment. |
|
Strange. Did I introduce an infinite loop in the Forth code somehow? Would adding one more include consume another billion cycles? Was it a fluke? Those changes are on extract-base. Shall I try a separate PR? |
|
Running out of cycles means, either the build or test process got stuck somewhere. |
|
Sorry about that, shiny button was there, wanted me to push it. It's back to where the test passed, without my extra merge commit. |
This reverts commit 0eee383. # Conflicts: # asm/durexforth.asm
|
You are right :-) At a minimum, those wordlists should be verified by the
build process.
Manual has same problem, there is no check that it is up to date.
Currently, durexForth developers are expected to grep.
lör 4 feb. 2023 kl. 01:06 skrev ekipan ***@***.***>:
… ***@***.**** commented on this pull request.
------------------------------
In asm/compiler.asm
<#514 (comment)>
:
> @@ -1,4 +1,4 @@
-; C, , ; IMMEDIATE [ ] STATE : HEADER LIT LITC COMPILE, LITERAL HERE DODOES
+; included into durexforth.asm
To play devil's advocate: by that token the word lists are also repeating
yourself. You can find that information by grepping BACKLINK.
I had two thoughts as a new reader. One, I wasn't sure where to start
reading. And two, when I see a label in some source file and I don't know
where it's from, it'd be nice to have a single index in one file instead of
needing to grep all the files. Maybe a developer should be expected to be
able to grep.
In any case several of them were out of date. Extra words, missing words.
—
Reply to this email directly, view it on GitHub
<#514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAY34O2JHEPTNEG6JH534BDWVWMOZANCNFSM6AAAAAAUQOGT4U>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Could I suggest a blurb to the effect of "Start reading asm/durexforth.asm. It includes a dozen asm files that defines the builtin words, and then loads forth/base.fs and starts interpreting, defining the rest of the vocabulary." Maybe in README or CONTRIBUTING? As a matter of preference, I'm not convinced that separate asm/ and forth/ directories buys anything. Personally I'd prefer a single src/ directory. Edit: I wish I'd submitted this from a branch not named |
|
It might be interesting for you to check out v1.25 tag? Thats how it used
to be a couple of years ago. A single assembly file (durexforth.a) in root
folder. That single-file structure definitely had its merits, it was very
simple and fast to search in. But it is pros and cons, I also think it is
tidy to keep related words grouped in files, like they are now.
Maybe moving durexforth.asm to the root folder would be enough to help to
find the ”starting point”?
About separate asm and forth folders. I think assembly and forth files are
so different, especially from a build perspective, that it is a reasonable
organization. But I see your point too. Out of curiosity, what text editor
are you using?
lör 4 feb. 2023 kl. 02:02 skrev ekipan ***@***.***>:
… Could I suggest a blurb to the effect of "Start reading
asm/durexforth.asm. It includes a dozen asm files that defines the builtin
words, and then loads forth/base.fs and starts interpreting, defining the
rest of the vocabulary." Maybe in README or CONTRIBUTING?
As a matter of preference, I'm not convinced that separate asm/ and forth/
directories buys anything. Personally I'd prefer a single src/ directory.
—
Reply to this email directly, view it on GitHub
<#514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAY34O76ITXNLYEGJM3EKFTWVWTB3ANCNFSM6AAAAAAUQOGT4U>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
FYI: I added #515 Thank you for the idea! |
|
Oh I use some Scintilla thing whose key binds grafted themselves onto my fingers years ago. I should probably learn a "real editor" but I feel like this kind of thing is mostly habit. Related to that: I have git configured autocrlf=false, and I noticed my clone of durexforth has mostly files with CR+LF in them. I usually prefer LF files. I think I noticed a couple anomalies? Git status kept telling me I changed random lines, perhaps someone else had edited the CRs out of them. Incidentally, what do you think of 0eee383...8742ace? Maybe you'd prefer to keep the macros together but do you want the extra prose? I'm closing this PR, gonna do some more surgery on my fork and don't want to spam your notifs. |
|
Umm... the .gitattributes file contains the line 0eee383...8742ace <-- looks fine to me! |
|
#516 made these changes stale, I redid them: ekipan:comments. If you'd like the prose I could make another PR. |
|
I am all for it, if you think it would help others!
lör 4 feb. 2023 kl. 18:56 skrev ekipan ***@***.***>:
… #516 <#516> made these
changes stale, I redid them: ekipan:df-cmts
<https://github.com/ekipan/durexforth/tree/df-cmts>. If you'd like the
prose I could make another PR.
—
Reply to this email directly, view it on GitHub
<#514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAY34O2NJJ5NUU55JKP53DDWV2J6JANCNFSM6AAAAAAUQOGT4U>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I wrote a few comments and moved a few things to help me as I start to read the source. Forgive my rudeness but I have no build environment, these changes are untested. The hope is that they're trivial enough to be without error, or at least easy enough to fix.
If you find them useful.