Skip to content
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

refactor rewriteJs and rewriteCoffee #180

Merged
merged 1 commit into from
Jan 25, 2024
Merged

refactor rewriteJs and rewriteCoffee #180

merged 1 commit into from
Jan 25, 2024

Conversation

davidchambers
Copy link
Owner

I made some of these internal changes a few years ago. 🙈

Here is the new algorithm (with slight variations due to differences in the way comments are located):

$ awk '$2 ~ /rewrite(Js|Coffee)/ { print $2; look = 1 } $0 == "};" { look = 0 } look && $1 == "//" && $2 ~ /:$/ { print }' lib/doctest.js
rewriteJs
  // 1: Parse source text to extract comments
  // 2: Preserve source text between comments
  // 3: Extract prefixed comment lines
  // 4: Coalesce related input and output lines
  // 5: Convert doctests to source text
  // 6: Sort verbatim and generated source text by original offsets
  // 7: Concatenate source text
rewriteCoffee
  // 1a: Extract prefixed comment lines
  // 1b: Preserve other lines
  // 2: Coalesce related input and output lines
  // 3: Convert doctests to source text
  // 4: Sort verbatim and generated source text by original line numbers
  // 5: Concatenate source text

The change to the test suite reflects the removal of transformComments and substring and their accompanying doctests.

Copy link
Collaborator

@Avaq Avaq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a superficial review and this LGTM: You say it's just a refactor, the code looks alright, the number of lines has been halved, and the tests still pass. If I need to do a more thorough review let me know.

@davidchambers davidchambers merged commit 45b327b into main Jan 25, 2024
6 checks passed
@davidchambers davidchambers deleted the refactor branch January 25, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants