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

Adds Extracted PLI Compiler Messages/Codes #6

Open
wants to merge 2 commits into
base: feature/langium-project
Choose a base branch
from

Conversation

montymxb
Copy link
Member

@montymxb montymxb commented Dec 5, 2024

Adds in what was extracted from the 6.1 compiler messages & codes document. Much in the same fashion as tests were extracted, there's over 1k codes, along with detailed messages & explanations. This pulls it all together into a TS file that we can reference with full information intact, ideally making it easier as we start to work these into validations.

Heads-up, the synthesized codes file is over 18k lines long. It's a bit large up front, but we're going to, eventually, need all of these for full support down the road.

One validation is updated to show how this works in practice, the rest can be trivially updated as well. I was also able to auto-detect when params were likely needing to be substituted, and modified those cases to be parametric pli code objects. In that way, usage requires you to provide the params to complete the message as intended.

@montymxb montymxb requested a review from msujew December 5, 2024 16:08
@msujew msujew force-pushed the feature/langium-project branch from cfcd5bf to 6b5d434 Compare December 11, 2024 13:19
export const Info = {

/**
* This message is used in building the options listing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be helpful to have the explanation also embedded as a string.
Both message and explanation have a value for the user. The page number could be also stored as a number for a goto feature if someone wants this (just an idea).

Copy link
Member Author

@montymxb montymxb Jan 17, 2025

Choose a reason for hiding this comment

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

Do you have a case in mind for accessing the explanation as a string? I couldn't think of one when I was putting this together, so I would probably hold off until we know what we need it for. If a case does come up it's pretty easy to regenerate all of this, but also include that additional string prop, so it's fairly to include as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking of the message itself when hovering the diagnostic. Not necessarily in the printout/diagnostics of the error panel. IF this is even possible! Then, we could also store a link to the documentation. But I also see that we are good enough already. No real need for change here.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a pretty good idea actually, and that wouldn't be too hard to add in. I think might be a good follow-up to this one.

@@ -0,0 +1,18347 @@
/**
* Generated by process-codes-into-ts.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is the generator? Are there reasons to hide it? Would be great if we would be able to make the file also for future or other versions of the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

The generator is a hand rolled thing I wrote up for scraping data out of the 6.1 documents we've been working with. Technically it's two parts, first a python extraction script to pull the pdf content out, and a 2nd typescript processor (the one mentioned here). I'm not sure if I want to include them in the regular project, but it would still make sense to share them internally.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then: Let's put both parts into a folder in our "company drive". WDYT?
May add a comment here, where one could find the generator.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A middle way could be to check in the scraped JSON, so that we are able to generate new formats later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah let's start with sharing it internally internally and then move forward based on that. But we should ultimately indicate & share these tools that are needed to extract these details, since subsequent PL/I releases will need to have the process repeated.

@msujew msujew force-pushed the feature/langium-project branch 4 times, most recently from 183a00e to 91e9352 Compare January 16, 2025 13:18
Co-authored-by: Benjamin Wilson <[email protected]>
Co-authored-by: Markus Rudolph <[email protected]>
Signed-off-by: Mark Sujew <[email protected]>
@msujew msujew force-pushed the feature/langium-project branch from 91e9352 to 4e07d32 Compare January 16, 2025 13:19
@montymxb montymxb force-pushed the feature/montymxb/compiler-codes branch from 3ba6786 to 476cfe5 Compare January 17, 2025 09:46
Copy link
Collaborator

@Lotes Lotes left a comment

Choose a reason for hiding this comment

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

Keeping my points in mind for future PRs, we can merge this as soon as our actual "master" branch is ready to be adjusted.

@msujew msujew force-pushed the feature/langium-project branch from 4e07d32 to 279a71d Compare January 20, 2025 13:07
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.

3 participants