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

[Discussion] directive has ambiguous syntax #1236

Open
VeryEarly opened this issue Sep 11, 2023 · 2 comments
Open

[Discussion] directive has ambiguous syntax #1236

VeryEarly opened this issue Sep 11, 2023 · 2 comments
Assignees

Comments

@VeryEarly
Copy link
Contributor

for where statement, following have ambiguous syntax

      const subjectRegex = getPatternToMatch(directive.where.subject);
      const subjectPrefixRegex = getPatternToMatch(directive.where['subject-prefix']);
      const verbRegex = getPatternToMatch(directive.where.verb);
      const variantRegex = getPatternToMatch(directive.where.variant);
      const parameterRegex = getPatternToMatch(directive.where['parameter-name']);
      const enumNameRegex = getPatternToMatch(directive.where['enum-name']);
      const enumValueNameRegex = getPatternToMatch(directive.where['enum-value-name']);
      const modelNameRegex = getPatternToMatch(directive.where['model-name']);
      const modelFullNameRegex = getPatternToMatch(directive.where['model-fullname']);
      const modelNamespaceRegex = getPatternToMatch(directive.where['model-namespace']);
      const propertyNameRegex = getPatternToMatch(directive.where['property-name']);

the method "getPatternToMatch" treat "pattern" as /^pattern$/ and "pattern." as /pattern./
for a regex, /pattern/ has different meaning from /^pattern$/, autorest.powershell should not make such assumption.

We should treat all input as regex.

@VeryEarly VeryEarly self-assigned this Sep 11, 2023
@VeryEarly VeryEarly changed the title directive has ambiguous syntax [BUG] directive has ambiguous syntax Sep 11, 2023
@dolauli dolauli changed the title [BUG] directive has ambiguous syntax [Discussion] directive has ambiguous syntax Sep 12, 2023
@dolauli
Copy link
Contributor

dolauli commented Sep 12, 2023

Let use https://github.com/Azure/autorest.powershell/blob/main/docs/directives.md#cmdlet-hiding-exportation-suppression for example.
Current design:
Filters without special characters, e.g. verb: update, it will be implemented /^update$/ instead of /update/ for exactly word match.
Filters with special characters, e.g. subject: PetService.*, it will be implemented as /PetService.*/
Limitation of the design, if someone want to filter all the verb that contains update, he will need to use something like verb: update\d*

@VeryEarly's suggestion is in both cases, we just use regex. And for this solution, for an exact match, developers will need to change the directive from verb: update to verb: ^update$

@dolauli
Copy link
Contributor

dolauli commented Sep 14, 2023

Considering it is a breaking change and the impact is low, it is not worth to make change. Will need to update the docs to make the current design clear to all the users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants