{"action":"created","issue":{"url":"https://api.github.com/repos/martanne/vis/issues/558","repository_url":"https://api.github.com/repos/martanne/vis","labels_url":"https://api.github.com/repos/martanne/vis/issues/558/labels{/name}","comments_url":"https://api.github.com/repos/martanne/vis/issues/558/comments","events_url":"https://api.github.com/repos/martanne/vis/issues/558/events","html_url":"https://github.com/martanne/vis/pull/558","id":230234624,"number":558,"title":"Implementation of the `gf` command, v2 (RFC)","user":{"login":"Shugyousha","id":500068,"avatar_url":"https://avatars0.githubusercontent.com/u/500068?v=3","gravatar_id":"","url":"https://api.github.com/users/Shugyousha","html_url":"https://github.com/Shugyousha","followers_url":"https://api.github.com/users/Shugyousha/followers","following_url":"https://api.github.com/users/Shugyousha/following{/other_user}","gists_url":"https://api.github.com/users/Shugyousha/gists{/gist_id}","starred_url":"https://api.github.com/users/Shugyousha/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shugyousha/subscriptions","organizations_url":"https://api.github.com/users/Shugyousha/orgs","repos_url":"https://api.github.com/users/Shugyousha/repos","events_url":"https://api.github.com/users/Shugyousha/events{/privacy}","received_events_url":"https://api.github.com/users/Shugyousha/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2017-05-21T17:19:54Z","updated_at":"2017-06-18T17:10:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/martanne/vis/pulls/558","html_url":"https://github.com/martanne/vis/pull/558","diff_url":"https://github.com/martanne/vis/pull/558.diff","patch_url":"https://github.com/martanne/vis/pull/558.patch"},"body":"This is a rebased continuation of PR https://github.com/martanne/vis/pull/502. I have not closed the other one yet because we may have to cross-reference some things from there.\r\n\r\nThis implementation uses lpeg to detect the filename and the offset pattern. The implementation is still dependent on the text_object_longword function and I am not sure how we can get rid of this dependency. Even when using lpeg we have to know where to start searching for the lpeg pattern. Since the current cursor position could be in the middle of a file name, we have a sort of chicken-and-egg problem. We need a way to find out where the filename begins before we can start searching for the file name lpeg pattern...\r\n\r\nIn the earlier implementation we discussed reusing the sam.c  parser (potentially through vis:command) in order to parse the offset string after the file name. I do the (simplified) parsing using lpeg in the lua code because we would have to find at least the beginning and end of the \"filename + offset\" string there anyways before we can pass it to vis:command.\r\n\r\nThe current version also parses offset patterns like 'filename.c:/somestring'. The '/somestring', if found, is passed to vis:command which causes vis to move the cursor to the first occurrence of 'somestring' in the file (which we want) and then changes to visual mode (which we don't want).\r\n\r\nI haven't yet implemented the parsing of quoted filenames. I assume the parser should parse something like \"file with horrible spaces.c\":123 and not \"file with horrible spaces.c:123\" (or both?).\r\n\r\n\r\nOpen questions:\r\n\r\n* Should the Address parsing code in sam.c be used (through vis:command) to do the parsing or is lpeg sufficient?\r\n* How should we move the cursor to the first occurrence of \"pattern\"? Do we have to expose some search functionality through lua (if not available already)?\r\n* Do we want to merge a (cleaned-up) version of this implementation already or should we first decide on the points above and then implement another version?\r\n* Should we replace other filename parsing code with a shared lua implementation?"},"comment":{"url":"https://api.github.com/repos/martanne/vis/issues/comments/309290099","html_url":"https://github.com/martanne/vis/pull/558#issuecomment-309290099","issue_url":"https://api.github.com/repos/martanne/vis/issues/558","id":309290099,"user":{"login":"Shugyousha","id":500068,"avatar_url":"https://avatars0.githubusercontent.com/u/500068?v=3","gravatar_id":"","url":"https://api.github.com/users/Shugyousha","html_url":"https://github.com/Shugyousha","followers_url":"https://api.github.com/users/Shugyousha/followers","following_url":"https://api.github.com/users/Shugyousha/following{/other_user}","gists_url":"https://api.github.com/users/Shugyousha/gists{/gist_id}","starred_url":"https://api.github.com/users/Shugyousha/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shugyousha/subscriptions","organizations_url":"https://api.github.com/users/Shugyousha/orgs","repos_url":"https://api.github.com/users/Shugyousha/repos","events_url":"https://api.github.com/users/Shugyousha/events{/privacy}","received_events_url":"https://api.github.com/users/Shugyousha/received_events","type":"User","site_admin":false},"created_at":"2017-06-18T17:10:16Z","updated_at":"2017-06-18T17:10:16Z","body":"On Tue, May 30, 2017 at 01:02:28PM -0700, Marc André Tanner wrote:\n> * Can you use pure lpeg patterns instead of relying on the `re`\n>   module? You can also reuse some [patterns provided by the lexermodule](https://github.com/martanne/vis/blob/master/lua/lexers/lexer.lua#L1303).\n\nOk, I can use something like this\n\nlocal l = require('lexer')\nlocal lpeg = vis.lpeg\nlocal namepat = l.any - lpeg.S('\"\\'\\t\\v\\f\\n\\r ')\n\nThis still doesn't cover the quoted-filename case but I can use\n`delimited_range` for that.\n\n> * Instead of the text object function try using\n>   [`file:match_at`](http://martanne.github.io/vis/doc/#File:match_at). That\n>   should allow you to use things like the `delimited_range` function of\n>   the lexer module.\n\nI don't think match_at will work here. match_at expects to be given a\nposition to start searching at, a pattern to search for and an optional\nsearch horizon. If the cursor is within a file name and you pass a search\nhorizon that is larger than the number of characters between the cursor\nposition and the start of the filename, you will miss the file name and\nmay end up matching the preceding token because the search will always\nstart at pos - horizon. If your chosen search horizon is too small\nhowever you won't match the whole file name.\n\nThe only way I could think of to make sure to match the whole file name\nis to move the cursor back from pos to the beginning of the potential\nfilename (by checking each preceding character if it may be part of the\nfile name or not).\n\nAlso, when using match_at, it wouldn't be possible to both match the\nfilename and the offset in one call since the function only returns\none range (and not the start and end of both the filename part and the\naddress-within-the-file part like we do with the current :match call).\n\n\n> Regarding the parsing, how does `acme` separate the file name from a\n> possible address?\n\nI looked into it through\n\nhttps://github.com/Harvey-OS/harvey/blob/6533db6114e97d5687bdb6a0f94136c98d009408/sys/src/cmd/acme/edit.c\n\nand\n\nhttps://github.com/Harvey-OS/harvey/blob/52a059d54222c030241de2359803d1d6d7a40ecb/sys/src/cmd/acme/ecmd.c\n\n(Harvey OS is the only Plan9 source I could find that was still online)\n\nbut I had issues determing how the file name is being parsed. The parsing\nof the address can be found at\n\nhttps://github.com/Harvey-OS/harvey/blob/52a059d54222c030241de2359803d1d6d7a40ecb/sys/src/cmd/acme/ecmd.c#L998\n\nSome of the possible Acme filename addresses are shown here:\n\nhttps://youtu.be/dP1xVpMPn8M?t=9m9s\n\nI strongly suspect that Acme just consideres any string up to a space\nchar as a file name but I am not sure.\n\nDo you want to support quoted filenames? If so, do you want to support\n\n\"file with horrible spaces.c\":123\n\nor\n\n\"file with horrible spaces.c:123\"\n\nor both?\n\n"}}
{"actor":{"display_login":"Shugyousha"}}