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

Complete roundtrip of PL <--> OPB MD conversion #87

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

Bluesy1
Copy link
Collaborator

@Bluesy1 Bluesy1 commented Aug 11, 2023

Complete the PL to/from OPB MD roundtrip

TODO:

  • Add support for asset files
  • Reorganize new imports to their own section instead of at the top of the import list
  • Add parameter for output destination and write file(s) generated to destination
    • Also copy back asset files
  • Add test cases (expected outputs -> markdown files)
  • Update test action to upload generated return trip outputs
  • Add documentation elsewhere in the file to note cases when new features are added will require the parts of the parser to be updated to match compatibility
  • Add support for keys that cannot be roundtripped (waiting on progress on Finish the roundtrip: PL --> MD conversions #86 for this)

Resolves #86

Does not try to retain assets, or defined asset values, and loses the types of non string customizations currently.
@Bluesy1 Bluesy1 self-assigned this Aug 11, 2023
- Add missing section to md result we cannot roundtrip right now
- Move imports to a dedicated sections with a comment
- Using `ast.literal_eval`, customizations that are literals are parsed back from strings to their native type
now attempts to list assets found in the 4 key folders from prairielearn
Need to flatten the namedtuple to a list
- Now copies asset files
- Allows for an output directory and filename to be passed
- Validates input paths are directories and not files
- Improves argument checking
@Bluesy1
Copy link
Collaborator Author

Bluesy1 commented Aug 12, 2023

Benchmark of what output looks like as of August 11th, 2023: https://gist.github.com/Bluesy1/6f497bc1ed5b4a3aae25f91918abe2ad

- Need to prevent `"..."` from being evaled to the `Ellipses` literal
- Remove the `__pycache__` directory created by importing and executing the `server.py` file
- Fix `server.py` parsing
- Fix asset copying
- Add `pl-rich-text-editor` to list of supported pl inputs
Also minor updates to overall test layout to make round trip testing work nicer
Now should upload md generated from pl files on failure
…metadata field of the comment object in `info.json`
@Bluesy1 Bluesy1 marked this pull request as ready for review August 18, 2023 02:56
@Bluesy1 Bluesy1 requested a review from firasm August 18, 2023 03:00
@Bluesy1
Copy link
Collaborator Author

Bluesy1 commented Aug 18, 2023

For now, to make the roundtrip possible, I've shoved all the extra keys I can't get normally into a nested object inside the comment section of the info.json generated files to at least see how that would look - I'm happy to change and/or modify the method if needed, or just accept that those sections have to be manually copied back (reverted to prior state?), by a human.

I expect this will need multiple rounds of review since its a fairly large change, but it should be mostly feature complete now and working.

@Bluesy1 Bluesy1 changed the title [WIP] Complete roundtrip of PL <--> OPB MD conversion Complete roundtrip of PL <--> OPB MD conversion Aug 18, 2023
@Bluesy1 Bluesy1 mentioned this pull request May 27, 2024
@Bluesy1 Bluesy1 marked this pull request as draft June 3, 2024 00:48
@Bluesy1 Bluesy1 marked this pull request as ready for review June 3, 2024 02:22
@Bluesy1 Bluesy1 force-pushed the main branch 2 times, most recently from 5d27a03 to 5e18acf Compare June 19, 2024 01:30
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.

Finish the roundtrip: PL --> MD conversions
1 participant