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

Revisit Characters Allowed in Identifiers #41

Closed
zdne opened this issue Nov 13, 2013 · 4 comments
Closed

Revisit Characters Allowed in Identifiers #41

zdne opened this issue Nov 13, 2013 · 4 comments

Comments

@zdne
Copy link
Contributor

zdne commented Nov 13, 2013

Identifiers limitations tend to feel way too restrictive.

Reduce the non allowed characters to minimum. This should allow use of unicode (national) characters in Resource and Action identifiers. Consider banning [ and ] only.

@zie1ony
Copy link

zie1ony commented Feb 5, 2014

After taking a look on:

I started experiment with regex I found in SymbolTable.h. Currently it's: "[[:alnum:][:blank:]_-\']+". According to tip to ban only [ and ] I changed it to: [^]^[^\\n]+ witch blocks ], [ and \n.
Then I did some testing:

FORMAT: 1A

# Group Parcel

## Parcel's sticker [/asd]
desc

compiles to:

ParagraphBlockType, content: 'FORMAT: 1A', :0:12
HeaderBlockType, content: 'Group Parcel', :12:16
HeaderBlockType, content: 'Parcel's sticker [/asd]', :28:27
ParagraphBlockType, content: 'desc', :55:6

{
  "_version": "1.0",
  "metadata": {
    "FORMAT": {
      "value": "1A"
    }
  },
  "name": "",
  "description": "",
  "resourceGroups": [
    {
      "name": "Parcel",
      "description": "",
      "resources": [
        {
          "name": "Parcel's sticker",
          "description": "desc\n\n",
          "uriTemplate": "/asd",
          "model": {},
          "parameters": {},
          "headers": {},
          "actions": []
        }
      ]
    }
  ]
}
RMAT: 1A

# Russian testing

## Категории [/categories]

### Категории [GET]

+ Response 200 (application/json)

        [{}]

compiles to:

ParagraphBlockType, content: 'RMAT: 1A', :0:10
HeaderBlockType, content: 'Russian testing', :10:19
HeaderBlockType, content: 'Категории [/categories]', :29:37
HeaderBlockType, content: 'Категории [GET]', :66:30
ListBlockBeginType
ListItemBlockBeginType
ParagraphBlockType, content: 'Response 200 (application/json)', :98:33
CodeBlockType, content: '[{}]\n', :135:9
ListItemBlockEndType, :98:33;135:9
ListBlockEndType, :96:49

{
  "_version": "1.0",
  "metadata": {
    "RMAT": {
      "value": "1A"
    }
  },
  "name": "Russian testing",
  "description": "",
  "resourceGroups": [
    {
      "name": "",
      "description": "",
      "resources": [
        {
          "name": "Категории",
          "description": "",
          "uriTemplate": "/categories",
          "model": {},
          "parameters": {},
          "headers": {},
          "actions": [
            {
              "name": "Категории",
              "description": "",
              "method": "GET",
              "parameters": {},
              "headers": {},
              "examples": [
                {
                  "name": "",
                  "description": "",
                  "requests": [],
                  "responses": [
                    {
                      "name": "200",
                      "description": "",
                      "headers": {
                        "Content-Type": {
                          "value": "application/json"
                        }
                      },
                      "body": "[{}]\n",
                      "schema": ""
                    }
                  ]
                }
              ]
            }
          ]
        }
      ]
    }
  ]
}

Are these translations correct?

@zdne
Copy link
Contributor Author

zdne commented Feb 5, 2014

@zie1ony

Looks good!

Note I would omit the stopping \n completely (or consider using $). It would be also good to address apiaryio/snowcrash#70 along with this fix.

Please follow with pull requests both on Snow Crash and section 3.6 of the API Blueprint Specification.

Thank you!

@zie1ony
Copy link

zie1ony commented Feb 5, 2014

Not so fast. I ran make test and there were few fails :( Same thing when I changed identifier's regex to [^]^[ ]+ or [^]^[^$] or [^]^[^\\$]. I started to debug tests, but it is not so easy to figure out what is going on deep down in the stack...

@zdne
Copy link
Contributor Author

zdne commented Feb 5, 2014

@zie1ony

Careful about POSIX regex and escaping. The \ is not an escape character within the group!

Also because of the Media Type definition after symbol (as in https://github.com/apiaryio/snowcrash/blob/master/src/PayloadParser.h#L29) we will need to unfortunately exclude ( and ) as well.

So you might end up with something like

#define SYMBOL_IDENTIFIER "([^][()]+)"

After that only test-PayloadParser.cc:681 should be failing which will actually become false positive since we have changed allowed characters.

@zdne zdne closed this as completed in c2c1d3e Feb 7, 2014
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

No branches or pull requests

2 participants