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

Example of displayed value on hover of NUMERIC-EDITED variables #337

Merged
merged 7 commits into from
Aug 8, 2024

Conversation

NeoKaios
Copy link
Contributor

@NeoKaios NeoKaios commented Aug 5, 2024

Closes #296
Note that picture string with P and E character are not yet implemented

@nberth
Copy link
Collaborator

nberth commented Aug 5, 2024

I guess at the moment adding lsp = ">=1.18 & <1.19" in the cobol_lsp/package.toml should do fine. We upgrading that should be kept for a separate PR.

@nberth nberth closed this Aug 5, 2024
@nberth nberth reopened this Aug 5, 2024
@NeoKaios NeoKaios force-pushed the feat/pic-example-on-hover branch 2 times, most recently from efca4cf to 094d3e5 Compare August 6, 2024 10:42
test/lsp/lsp_hover.ml Outdated Show resolved Hide resolved
test/lsp/lsp_hover.ml Outdated Show resolved Hide resolved
@NeoKaios NeoKaios force-pushed the feat/pic-example-on-hover branch from c741f5f to f60f2f5 Compare August 8, 2024 08:02
Copy link
Collaborator

@nberth nberth left a comment

Choose a reason for hiding this comment

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

Good job!

Just a small question: is P really still not supported (it does not appear to be tested in test/lsp/lsp_picture_interp.ml)? About E, what are the current limitations and what happens on such items?

@NeoKaios
Copy link
Contributor Author

NeoKaios commented Aug 8, 2024

P are still not supported:
I think there is an issue/missing info when building the category of vars whose picture string contains Ps
Because picture string V99 and P9 lead to the exact same category NUMERIC(digits = 2, scale = 2, with_sign = false)
(Also PP is valid in superbol even tho it's not a valid picture string.)
The handling of PIC 9P and similar should be doable, but needs to be done separately because of the scale = -1

I didn't do E because I only found 1 example in the docs. Now that I think about it, I suppose that the implementation reflects scientific notation so it can be done, tho without feedback, as cobc issue an error when an E is present libcob: error: cob_move_display_to_edited: invalid PIC character E

Atm, items with E and P simply do not have a e.g clause

@nberth
Copy link
Collaborator

nberth commented Aug 8, 2024

P are still not supported: I think there is an issue/missing info when building the category of vars whose picture string contains Ps Because picture string V99 and P9 lead to the exact same category NUMERIC(digits = 2, scale = 2, with_sign = false) (Also PP is valid in superbol even tho it's not a valid picture string.) The handling of PIC 9P and similar should be doable, but needs to be done separately because of the scale = -1

In itself it's not an issue if two distinct picture strings lead to the same representation (there's no requirement like this in the language). Maybe it's actually just a missing post-scan check for rejecting P+ (the regexp) as picture strings so we don't wrongfully assign them a meaning (there are other ad hoc "post-scan" checks like this, because of the way I wrote the scanning of picture strings). Also note GnuCOBOL has issues with some picture strings due to the way it scans them (but I don't remember which ones; there may be a bug report somewhere about that), so it's not an ideal reference.

I didn't do E because I only found 1 example in the docs. Now that I think about it, I suppose that the implementation reflects scientific notation so it can be done, tho without feedback, as cobc issue an error when an E is present libcob: error: cob_move_display_to_edited: invalid PIC character E

Ah indeed it's possibly not supported yet. So we can ignore those for now.

Atm, items with E and P simply do not have a e.g clause

Ok fine with that.

@nberth nberth merged commit 4b3926a into OCamlPro:master Aug 8, 2024
4 checks passed
@NeoKaios NeoKaios deleted the feat/pic-example-on-hover branch August 9, 2024 11:58
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.

Improve the information shown when hovering data fields of -EDITED category
2 participants