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

Improve dump option support #344

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Conversation

chqrlie
Copy link
Collaborator

@chqrlie chqrlie commented Apr 4, 2024

  • add JS_SetDumpFlags() to select active dump options
  • accept -d[] and --dump[=] to specify active dump options, generalize command line option handling
  • make all defined DUMP_XXX output dynamically selectable
  • improve DUMP_READ_OBJECT output, fix indentation issue

Copy link
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Left some comments!

CMakeLists.txt Outdated Show resolved Hide resolved
qjs.c Show resolved Hide resolved
qjs.c Show resolved Hide resolved
quickjs.h Outdated Show resolved Hide resolved
quickjs.c Show resolved Hide resolved
quickjs.h Outdated Show resolved Hide resolved
quickjs.c Outdated Show resolved Hide resolved
quickjs.c Outdated Show resolved Hide resolved
quickjs.c Outdated Show resolved Hide resolved
quickjs.c Outdated Show resolved Hide resolved
@chqrlie chqrlie requested a review from saghul April 6, 2024 22:01
@saghul
Copy link
Contributor

saghul commented Apr 8, 2024

This needs a rebase now that you did the split and the other stuff can be merged.

@chqrlie
Copy link
Collaborator Author

chqrlie commented Apr 8, 2024

This needs a rebase now that you did the split and the other stuff can be merged.

Yes, I am aware of this and it would have been difficult to avoid it.
Can I just rebase my branch on master and push again?

@saghul
Copy link
Contributor

saghul commented Apr 8, 2024

Yes you can do that. Make sure to use --force then.

- DUMP_XXX defined as nothing or 0 produces unconditional output
- DUMP_XXX defined as a bitmask produces conditional output based
    on command line option -d<bitmask>
- add `JS_SetDumpFlags()` to select active dump options
- accept -d[<hex mask>] and --dump[=<hex mask>] to specify active
    dump options, generalize command line option handling
- improve DUMP_READ_OBJECT output, fix indentation issue
@chqrlie
Copy link
Collaborator Author

chqrlie commented Apr 9, 2024

@bnoordhuis do you have any remarks? can I proceed and merge this?

@chqrlie chqrlie merged commit 16e7661 into quickjs-ng:master Apr 14, 2024
46 checks passed
@chqrlie chqrlie deleted the improve-dump-options branch April 14, 2024 10:05
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