{"action":"created","issue":{"url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/110","labels_url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/110/labels{/name}","comments_url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/110/comments","events_url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/110/events","html_url":"https://github.com/jonathandelgado/SublimeTodoReview/pull/110","id":66498895,"number":110,"title":"Abstract keyword patterns","user":{"login":"ErichDonGubler","id":658538,"avatar_url":"https://avatars.githubusercontent.com/u/658538?v=3","gravatar_id":"","url":"https://api.github.com/users/ErichDonGubler","html_url":"https://github.com/ErichDonGubler","followers_url":"https://api.github.com/users/ErichDonGubler/followers","following_url":"https://api.github.com/users/ErichDonGubler/following{/other_user}","gists_url":"https://api.github.com/users/ErichDonGubler/gists{/gist_id}","starred_url":"https://api.github.com/users/ErichDonGubler/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ErichDonGubler/subscriptions","organizations_url":"https://api.github.com/users/ErichDonGubler/orgs","repos_url":"https://api.github.com/users/ErichDonGubler/repos","events_url":"https://api.github.com/users/ErichDonGubler/events{/privacy}","received_events_url":"https://api.github.com/users/ErichDonGubler/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":6,"created_at":"2015-04-06T00:44:47Z","updated_at":"2015-04-09T16:27:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/pulls/110","html_url":"https://github.com/jonathandelgado/SublimeTodoReview/pull/110","diff_url":"https://github.com/jonathandelgado/SublimeTodoReview/pull/110.diff","patch_url":"https://github.com/jonathandelgado/SublimeTodoReview/pull/110.patch"},"body":"## Motivation\r\n\r\nI am submitting this PR in light of discussion after my submission of [issue #109](https://github.com/jonathandelgado/SublimeTodoReview/issues/109).\r\n\r\nIn essence, I would like to use code to the left of my comment as the note for the todo. You are forced to use different patterns if you want to do this:\r\n\r\n```c++\r\nclass Something //TODO: Make all members private!\r\n//...\r\nfriend std::ostream& operator<<(std::ostream& os, const MyClass& mc); //TODO\r\n```\r\n\r\nThe problem can only be approached with what I would call a hack. Python does not permit multiple definitions of a named capture group, so I'm forced to use a different name entirely for what is effectively the same thing (i.e., `(?P<todo_back>)`). In this PR, I have combined this with the separation of abstract overall patterns from keywords to make something I feel to be a better configuration design. It solves the problem, and frankly is more intuitive and therefore less prone to error.\r\n\r\n## Thought\r\n\r\nIn thinking of how to design this fix, best way I imagined to resolve this would be change design. Consider the following config after adding my own custom keywords:\r\n\r\n```json\r\n{\r\n\t\"patterns\":\r\n\t{\r\n\t\t\"TODO\": \"TODO[\\\\s]*:[\\\\s]*(?P<todo>.*)$\",\r\n\t\t\"TEST\": \"TEST[\\\\s]*:[\\\\s]*(?P<test>.*)$\",\r\n\t\t\"FIXME\": \"FIX ?ME[\\\\s]*:[\\\\s]*(?P<fixme>.*)$\",\r\n\t\t\"XXX\": \"XXX[\\\\s]*:[\\\\s]*(?P<xxx>.*)$\",\r\n\t\t\"DOCUMENT\": \"DOC(?:UMENT)?[\\\\s]*:[\\\\s]*(?P<document>.*)$\",\r\n\t\t\"DEBUG\": \"DEBUG[\\\\s]*:[\\\\s]*(?P<debug>.*)$\",\r\n\t\t\"REMOVE\": \"REMOVE[\\\\s]*:[\\\\s]*(?P<remove>.*)$\"\r\n\t},\r\n\t\"patterns_weight\":\r\n\t{\r\n\t\t\"FIXME\": 4,\r\n\t\t\"XXX\": 6,\r\n\t\t\"TODO\": 7\r\n\t}\r\n}\r\n```\r\n\r\n### Observations\r\n \r\n* The patterns are repetitive. I find it very unlikely that users will want to vary much from the overall pattern with individual keywords *(please correct me if I'm wrong!)*. The pattern provided by vanilla config is basically of the form `(?P<keyword>\\\\w+)\\\\s*:\\\\s*(?P<note>.*)$`.\r\n* I have different config sections for different properties of the same keyword (`patterns` and the weights in `patterns_weight`). Design principles guide us to separate what changes from what stays the same.\r\n\r\n## Solution \r\n\r\nIt seems to me that one could achieve a more cohesive result by associating attributes to individual keywords with a map, which are plugged into the abstract patterns.\r\n\r\nI think a good design to handle this could be as follows:\r\n\r\n1. Change `patterns` to a list of abstract patterns from current dictionary of keywords and their associated patterns. Use group names for different fields of the abstract pattern, the only two of which I can think of right now are `keyword` and `note`.\r\n2. Recognize user definitions of todo categories with a `keywords` dictionary. Expect category names to map to their own configuration dictionaries, thus having a cohesive area for properties like `weight` and a custom `pattern` for the keyword.\r\n\r\nStarting with something we recognize, the equivalent of the default configuration would be:\r\n\r\n```json\r\n{\r\n\t\"patterns\": [\r\n\t\t\"(?P<keyword>\\\\w+)\\\\s*:\\\\s*(?P<note>.*)$\"\r\n\t],\r\n\t\"keywords\": {\r\n\t\t\"TODO\": {}\r\n\t}\r\n}\r\n```\r\n\r\n...where  the `TODO` category uses the pattern `\"TODO\"` and has a weight of 100 (both by default, the former according to the [current commit](https://github.com/jonathandelgado/SublimeTodoReview/commit/184988a453ed3fa9d2ef42d20b375004ed285231)).\r\n\r\n\r\nExample configuration for weight and keyword pattern with new design:\r\n\r\n```json\r\n{\r\n\t\"patterns\":\r\n\t[\r\n\t\t\"(?P<keyword>\\\\w+)\\\\s*:\\\\s*(?P<note>.*)$\",\r\n\t\t\"^\\\\s*(?P<note>.*)(\\\\/\\\\/|#)\\\\s*(?P<keyword>\\\\w+)\\\\s*$\"\r\n\t],\r\n\t\"keywords\":\r\n\t{\r\n\t\t\"FIXME\":\r\n\t\t{\r\n\t\t\t\"pattern\": \"FIX(?:ME)?\",\r\n\t\t\t\"weight\": 4\r\n\t\t},\r\n\t\t\"XXX\":\r\n\t\t{\r\n\t\t\t\"weight\": 6\r\n\t\t},\r\n\t\t\"TODO\": {}\r\n\t}\r\n}\r\n```\r\n\r\nThe second line in the `patterns` list implements the motivation for this PR easily.\r\n\r\nThis would allow you do get this:\r\n\r\n```\r\n// domingo 04/05/15 at 06:35 - 1 files in 0.01 secs\r\n\r\n## FIXME (1)\r\n1. Desktop/asdf.d:3   void broken(){}\r\n\r\n## TODO (1)\r\n1. Desktop/asdf.d:7   Say something flashy!\r\n\r\n```\r\n\r\n...from example source file `asdf.d`:\r\n\r\n```d\r\nimport std.stdio;\r\n\r\nvoid broken(){}//FIXME\r\n\r\nint main(string[] args)\r\n{\r\n\twriteln(\"Hello, world!\");//TODO: Say something flashy!\r\n\treturn 0;\r\n}\r\n```\r\n\r\n### Other Consequences\r\n\r\n* Defining keyword-specific patterns is actually facilitated by this design also -- there's less copypasta than before. If you wanted to restrict my desired todo solution only to the keywords TODO and DOCUMENT, you'd do something like this:\r\n\r\n```json\r\n{\r\n\t\"patterns\":\r\n\t[\r\n\t\t\"(?P<keyword>\\\\w+)\\\\s*:\\\\s*(?P<note>.*)$\",\r\n\t\t\"^\\\\s*(?P<note>.*)(\\\\/\\\\/|#)\\\\s*(?P<keyword>TODO|DOC(?:UMENT)?)\\\\s*$\"\r\n\t],\r\n\t\"keywords\":\r\n\t{\r\n\t\t\"FIXME\":\r\n\t\t{\r\n\t\t\t\"pattern\": \"FIX(?:ME)?\",\r\n\t\t\t\"weight\": 4\r\n\t\t},\r\n\t\t\"XXX\":\r\n\t\t{\r\n\t\t\t\"weight\": 6\r\n\t\t},\r\n\t\t\"TODO\": {},\r\n\t\t\"DOCUMENT\":\r\n\t\t{\r\n\t\t\t\"pattern\": \"DOC(?:UMENT)?\",\r\n\t\t\t\"weight\": 1\r\n\t\t}\r\n\t}\r\n}\r\n```\r\n* Abstract design for overall todo patterns will immensely facilitate adding other fields to the TodoReview tab (like an explicit `(?P<code>)` field, or something)."},"comment":{"url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/comments/91281095","html_url":"https://github.com/jonathandelgado/SublimeTodoReview/pull/110#issuecomment-91281095","issue_url":"https://api.github.com/repos/jonathandelgado/SublimeTodoReview/issues/110","id":91281095,"user":{"login":"jonathandelgado","id":1841149,"avatar_url":"https://avatars.githubusercontent.com/u/1841149?v=3","gravatar_id":"","url":"https://api.github.com/users/jonathandelgado","html_url":"https://github.com/jonathandelgado","followers_url":"https://api.github.com/users/jonathandelgado/followers","following_url":"https://api.github.com/users/jonathandelgado/following{/other_user}","gists_url":"https://api.github.com/users/jonathandelgado/gists{/gist_id}","starred_url":"https://api.github.com/users/jonathandelgado/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonathandelgado/subscriptions","organizations_url":"https://api.github.com/users/jonathandelgado/orgs","repos_url":"https://api.github.com/users/jonathandelgado/repos","events_url":"https://api.github.com/users/jonathandelgado/events{/privacy}","received_events_url":"https://api.github.com/users/jonathandelgado/received_events","type":"User","site_admin":false},"created_at":"2015-04-09T16:27:24Z","updated_at":"2015-04-09T16:27:24Z","body":"@ErichDonGubler Alright, I figured out why I was matching `param.test(/match/);`, I was running on a previous config with a different pattern set. When I initially reviewed this PR, I didn't realize that you completely removed the pattern system as well, it looked initially like (to me) you merged the array of patterns with the array of keywords.\r\n\r\nWith that being said, there are a lot of changes here to the engine to fix one thing, like I mentioned I didn't want to happen in #109.\r\n\r\nPoints that must be fixed:\r\n- **Pattern system must be retained.** A lot of users use odd regexs to match non-traditional todo comments, myself included.\r\n- **Users config must not be broken.** I don't think the feature added here warrants making all users change their configs, consider using different setting key names.\r\n- **The priority system must be fixed.** As you noted, it was removed by accident.\r\n- **The speed must be comparable to what it is currently.** The speed that it is at right now is unacceptable, as you noted.\r\n- **Misc code formatting I mentioned.** Just removing the MIT author and code comments.\r\n\r\nI think ultimately, the only was this is going to be achieved, realistically, is rewriting the engine as I noted in #109, which I really don't want to do."}}