Improve onboarding

[ekipan]

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.

[ekipan]

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.

[jkotlinski]

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 https://github.com/jkotlinski/durexforth/commit/f449cf9e2d75b3946f9ea68c4a7bf8437a7e6f0f 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 https://github.com/jkotlinski/durexforth/pull/514#issuecomment-1416237085, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34OYAFZOMVYQ4SQMU2S3WVVD7JANCNFSM6AAAAAAUQOGT4U . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[ekipan]

I'm happy with the log. Please review.

[jkotlinski]

For some reason, the build does not pass with these changes. Any idea what might cause it?

[ekipan]

Looks like it ran out of cycles? I'm not sure why the rebased HEAD would if the old one didn't.

[ekipan]

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.

[ekipan]

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?

[jkotlinski]

Running out of cycles means, either the build or test process got stuck somewhere. Oh well, for whatever reason it worked now again. I will take a look at the changes in time :-)

[ekipan]

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.

[jkotlinski]

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 https://github.com/jkotlinski/durexforth/pull/514#discussion_r1096395416 :

@@ -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 https://github.com/jkotlinski/durexforth/pull/514#discussion_r1096395416, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34O2JHEPTNEG6JH534BDWVWMOZANCNFSM6AAAAAAUQOGT4U . You are receiving this because you commented.Message ID: @.***>

[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.

Edit: I wish I'd submitted this from a branch not named master. Oh well, if I want to experiment on my own it's not like the name master is special or anything.

[jkotlinski]

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 https://github.com/jkotlinski/durexforth/pull/514#issuecomment-1416570391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34O76ITXNLYEGJM3EKFTWVWTB3ANCNFSM6AAAAAAUQOGT4U . You are receiving this because you commented.Message ID: @.***>

[jkotlinski]

FYI: I added https://github.com/jkotlinski/durexforth/pull/515

Thank you for the idea!

[ekipan]

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.

[jkotlinski]

Umm... the .gitattributes file contains the line * text=auto, which I think means, text will be converted to LF automatically. You are right, it seems some files do not have normalized line endings. I am not sure how it came to be like that, or how to prevent it from happening again.

https://github.com/jkotlinski/durexforth/compare/0eee3839840622b8fa3be32220215f552132db6f...8742ace0e152d094630c65d339d5102dda13bc08 <-- looks fine to me!

[ekipan]

516 made these changes stale, I redid them: ekipan:comments. If you'd like the prose I could make another PR.

[jkotlinski]

I am all for it, if you think it would help others!

lör 4 feb. 2023 kl. 18:56 skrev ekipan @.***>:

516 https://github.com/jkotlinski/durexforth/pull/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 https://github.com/jkotlinski/durexforth/pull/514#issuecomment-1416812515, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY34O2NJJ5NUU55JKP53DDWV2J6JANCNFSM6AAAAAAUQOGT4U . You are receiving this because you commented.Message ID: @.***>