munificent / craftinginterpreters

Repository for the book "Crafting Interpreters"
http://www.craftinginterpreters.com/
Other
8.87k stars 1.04k forks source link

Whitespace formatting in `make c_chapters` generated files #1080

Closed quipa closed 2 years ago

quipa commented 2 years ago

I've noticed that files generated by make c_chapters miss newlines between function definitions in several files (not consistently though). I only mention this because it helps out when using diff to compare our code with yours to check for any blunders.

Here is in example in gen/chap24_call/chunk.c

// (...) headers
void initChunk(Chunk* chunk) {
  chunk->count = 0;
  chunk->capacity = 0;
  chunk->code = NULL;
  chunk->lines = NULL;
  initValueArray(&chunk->constants);
} // No newline in between
void freeChunk(Chunk* chunk) {
  FREE_ARRAY(uint8_t, chunk->code, chunk->capacity);
  FREE_ARRAY(int, chunk->lines, chunk->capacity);
  freeValueArray(&chunk->constants);
  initChunk(chunk);
}
// (...)

Not sure how hard this would be to change... Would altering the split_chapter tool code be enough? Or would the markup (//> //<) in the files themselves need to be changed..? Or maybe it would affect the formatting of the code snippets in the book..? No idea.

Personally, I got segmentation fault after the Closed upvalues section and I'm using diff to understand where I went wrong (segfault in the VM when executing an OP_CLOSURE instruction).

I managed to follow the chapter instructions "backwards" to get back to working code but took me quite a while (especially when lines are replaced, had to come and look in the repo)..! Since I am not using git to track changes (damn), I though of using diff.

As always, your book is great and when looking at all the markup in the c files, we really see the effort you put into it. Thanks! Can't wait to get back on track hehe...

quipa commented 2 years ago

Think I found a diff flag --ignore-blank-lines that can deal with this issue. Still makes the generated code look neater, but probably too much trouble for its worth. Think I'll close this issue, unless you find it relevant.