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

Associate context with a string #5

Closed
Ana06 opened this issue Jun 22, 2020 · 9 comments · Fixed by #39
Closed

Associate context with a string #5

Ana06 opened this issue Jun 22, 2020 · 9 comments · Fixed by #39
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Ana06
Copy link
Member

Ana06 commented Jun 22, 2020

@Ana06

In some cases it could be useful to associate context with a string as it can be done with numbers. For example:

- string: "{3E5FC7F9-9A51-4367-9063-A120244FBEC7}" = CLSID_CMSTPLUA

@mr-tz

hm, good point! maybe it makes sense to make the extra context available to all features.

@williballenthin

+1

@Ana06 Ana06 added enhancement New feature or request migrated labels Jun 22, 2020
@Ana06 Ana06 self-assigned this Jun 22, 2020
@Ana06
Copy link
Member Author

Ana06 commented Jun 26, 2020

We have discussed to support the following syntax:
- string: "'This program cannot be run in DOS mode.' = MS-DOS stub message"

I have been thinking about how to implementent htis and I think this brings some problems as well. What if the string or the description contains ' or "?

I am tending to think that it would be better/easier to allow a way to scape the =, for example \= or to only support it in regular expressions.

This proposal is easy to parse:

value, symbol = s.split(' = ', 1)
value.replace('\=', '=')

@williballenthin @mr-tz what do you think? Do you have a better idea to deal with ' or "?

@williballenthin
Copy link
Collaborator

williballenthin commented Jun 26, 2020

I think there's an edge case here:

value.replace('\=', '=')

what happens if the string should contain the literal \=?

@williballenthin
Copy link
Collaborator

what if we enabled string to also be a sequence of two elements, like:

- string:
  - This program can't be run in DOS mode
  - MS-DOS stub message

im not sure if this is legal, but might be more readable:

- string: This program can't be run in DOS mode
  - MS-DOS stub message

@williballenthin
Copy link
Collaborator

williballenthin commented Jun 26, 2020

image

image

@mr-tz
Copy link
Collaborator

mr-tz commented Jun 26, 2020

Hm, interesting. For consistency doing it the same way for all features would be good. If the parsing becomes to annoying or the syntax to cumbersome, this string / context (description) could work well:

- string: everything and anything
  description: my description

- number: 16
  description: a good number

@williballenthin
Copy link
Collaborator

that looks pretty good to me.

i still like the shorthand 16 = a good number, but it doesn't work in some cases (strings). we could do our best, and then use the linter to warn/fail when a term is ambiguous?

@mr-tz
Copy link
Collaborator

mr-tz commented Jun 26, 2020

In that case I'd say we go with 17 = also a good number for all features until someone break it with a good use-case 😃

@Ana06
Copy link
Member Author

Ana06 commented Jun 27, 2020

@williballenthin

what happens if the string should contain the literal =?

\\=, escaping the =

Maybe we never have a rule with an = in a string. I don't think that it is worthwhile to complicate the syntax for all features just for this case.

I have another proposal. What about splitting using the last = surrounded by spaces? which means that all characters (including =) are allowed in the string, but = surrounded by spaces is not allowed in the description. I can't come up with any case where a description should have an =. I could add a linter which complains if there are more than one and the type is not string (and maybe a warning for string).

@williballenthin
Copy link
Collaborator

from offline decision:

for all features but string, continue to support inline description using =. for string, the parsing is too complex to be worth it right now.

therefore, for all features, including string, also support description:.

the second form is the only way to describe a string. for other features, its use is discouraged, unless the description is long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants