GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Added option to pass custom C postprocessor by czgdp1807 · Pull Request #1331 · lcompilers/lpython

Conversation

@czgdp1807

@czgdp1807 czgdp1807 commented Nov 29, 2022

Copy link
Copy Markdown
Collaborator

cc: @certik

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
certik
certik reviewed Nov 29, 2022
View reviewed changes
Comment thread integration_tests/CMakeLists.txt Outdated
add_custom_command(
OUTPUT ${name}.c
COMMAND lpython --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
--custom-c-postprocessor ${CMAKE_CURRENT_SOURCE_DIR}/${custom_c_postprocessor} > ${name}.c

@certik certik Nov 29, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think as part of this change, let's not do --show-c, but rather save the result to a file ourselves, so no need for ">" which is quite fragile in build systems.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
czgdp1807 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@czgdp1807 czgdp1807 Nov 29, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I agree. Will do this on Thursday.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@czgdp1807 czgdp1807 Dec 5, 2022

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done. Please check the latest diff and let me know if you have any questions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@certik

certik commented Nov 29, 2022

Copy link
Copy Markdown
Contributor

Thanks for this, the other thing we need is to allow passing arguments to the script. I have just refactored such a script so that it accepts an argument of a some_file.inc that gets included at the top of the generated file as a header, instead of heaving the header hardwired in the script. I also need to add another argument with a file with a list of functions to modify.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@czgdp1807

czgdp1807 commented Dec 5, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

Well what do you think about this - Instead of passing a bunch of arguments to the post processor via LPython, can we just create a json file specifying all the configuration and the post processor can read this json file and do the necessary things. We can pass the path to this json file to the post processor via command line argument. So LPython will only call, python post_processor.py path_to_json_config_file and not python post_processor.py some_headers some_other_information some_more_information.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
czgdp1807 added 2 commits December 5, 2022 11:32
@czgdp1807
done
3c47c14
@czgdp1807
Write to a file if -o provided --show-c option
004a1af
@certik

certik commented Dec 5, 2022

Copy link
Copy Markdown
Contributor

Yes, the JSON approach is fine.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@certik

certik commented Dec 5, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I will test this PR manually and see if it satisfies our use case.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@certik certik mentioned this pull request Mar 23, 2023
LPython Roadmap #1600
Open
17 of 38 tasks
@czgdp1807 czgdp1807 closed this by deleting the head repository Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@certik certik certik left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@czgdp1807 @certik
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.