{"action":"created","issue":{"url":"https://api.github.com/repos/gollum/gollum/issues/1101","labels_url":"https://api.github.com/repos/gollum/gollum/issues/1101/labels{/name}","comments_url":"https://api.github.com/repos/gollum/gollum/issues/1101/comments","events_url":"https://api.github.com/repos/gollum/gollum/issues/1101/events","html_url":"https://github.com/gollum/gollum/pull/1101","id":123217314,"number":1101,"title":"Work in progress on Javascript editor","user":{"login":"AnvilStriker","id":12862883,"avatar_url":"https://avatars.githubusercontent.com/u/12862883?v=3","gravatar_id":"","url":"https://api.github.com/users/AnvilStriker","html_url":"https://github.com/AnvilStriker","followers_url":"https://api.github.com/users/AnvilStriker/followers","following_url":"https://api.github.com/users/AnvilStriker/following{/other_user}","gists_url":"https://api.github.com/users/AnvilStriker/gists{/gist_id}","starred_url":"https://api.github.com/users/AnvilStriker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AnvilStriker/subscriptions","organizations_url":"https://api.github.com/users/AnvilStriker/orgs","repos_url":"https://api.github.com/users/AnvilStriker/repos","events_url":"https://api.github.com/users/AnvilStriker/events{/privacy}","received_events_url":"https://api.github.com/users/AnvilStriker/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/gollum/gollum/labels/improvement","name":"improvement","color":"009800"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-12-21T06:16:53Z","updated_at":"2015-12-28T15:21:10Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/gollum/gollum/pulls/1101","html_url":"https://github.com/gollum/gollum/pull/1101","diff_url":"https://github.com/gollum/gollum/pull/1101.diff","patch_url":"https://github.com/gollum/gollum/pull/1101.patch"},"body":"In-progress changes in this branch (\"editor-functions-fix\"), not yet ready for merging:\r\n\r\n- Fix editor misfeatures related to \"backreferences,\" and loss of cursor position when applying functions that append text;\r\n- New editor functions for reStructuredText;\r\n- Introduction of QUnit testing framework, and a handful of tests (incomplete) for markdown and reStructuredText editor functions;\r\n- Sinatra endpoint for invoking tests.\r\n\r\nIf possible, I would like someone to review my approach and suggest improvements if anything I've done so far clashes with established design philosophy.\r\n\r\nThe small fixes to `gollum.editor.js`, and the editor functions in `langs/rest.js` are straightforward and should be unremarkable. (Note, though, that in the current commit I have left JS console debugging turned on in the editor, and have not updated the help text in `rest.js` -- it's still the help text for markdown, since I cloned `rest.js` from `markdown.js`, but haven't updated the text yet).\r\n\r\nOf greater possible concern is the way I've implemented the invocation of JS tests. In `app.rb` I've created a new endpoint called `jstest`, which takes an additional pathname element identifying the markup language whose test suite is to be run. For example:\r\n\r\n```\r\nhttp://localhost:4567/wiki/jstest/rest\r\n```\r\n\r\nruns the test suite for reStructuredText. Note that saving is disabled in the jstest editor view, although I haven't added the CSS necessary to \"grey out\" the \"Save\" button.\r\n\r\nRunning a test suite for a supported markup language that has no test suite defined results in \"0 of 0 tests passed.\" Running a test suite for an unsupported or unknown markup language results in a 404 Not Found. \r\n\r\nThis all seems to work quite nicely, but the question is whether this is really the _right_ way to invoke the tests, or if there's a better way that (a) doesn't use up a perfectly good endpoint name, and (b) could possibly run all defined test suites for supported languages at a single go.\r\n\r\n*** \r\n\r\nA second question, completely orthogonal to the first, is: \r\n\r\nDo the editor functions (across all the supported markup languages) work the way that a user would really want them to in terms of (a) whether they produce markup that will render correctly without further input from the user, and (b) where they leave the cursor or highlighted region?\r\n\r\nConsider the following cases:\r\n\r\n1. The \"HR\" function for markdown splits the current line at the cursor (or end of the current selection), and puts a sequence of three asterisks on a line by themselves at the split. However, it only puts a single newline before and after the asterisks, meaning that if the user goes immediately to the preview, the asterisks don't get rendered as a horizontal rule if there wasn't a newline immediately before the cursor position (or end of the selection) when the function was invoked. In this case the user needs to manually add the newline before the horizontal rule will render correctly. Should the HR function instead ensure that there is always a blank line before the asterisks? And if so, how sophisticated should it be? Is it okay to blindly add the extra newline even if there had already been a newline before the cursor? (See my implementation of \"HR\" for reStructuredText for an example of such a \"blind\" approach.)\r\n\r\n2. The \"HR\" function also has the annoying property that it moves the cursor all the way to the end of the editor buffer. I've tentatively fixed this by changing any \"append-only\" editor function to put the cursor at the _end_ of the appended fragment of text. But is this the right behavior? Might it make more sense to set the current selection to the entirety of the appended (so, for example, if the user is not happy with it, it's a simple matter of backspacing to get rid of it).\r\n\r\n3. See also the behavior of the UL and OL functions for both markdown and reStructuredText, which can also produce results that need the same kind of additional user input as in case 1 above in order to yield correct rendering.\r\n\r\nWould be curious to know if there's a \"desired behavior\" for these aspects of the editor functions, perhaps already implemented by some existing editor, that I should be trying to emulate."},"comment":{"url":"https://api.github.com/repos/gollum/gollum/issues/comments/167587145","html_url":"https://github.com/gollum/gollum/pull/1101#issuecomment-167587145","issue_url":"https://api.github.com/repos/gollum/gollum/issues/1101","id":167587145,"user":{"login":"dometto","id":147111,"avatar_url":"https://avatars.githubusercontent.com/u/147111?v=3","gravatar_id":"","url":"https://api.github.com/users/dometto","html_url":"https://github.com/dometto","followers_url":"https://api.github.com/users/dometto/followers","following_url":"https://api.github.com/users/dometto/following{/other_user}","gists_url":"https://api.github.com/users/dometto/gists{/gist_id}","starred_url":"https://api.github.com/users/dometto/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dometto/subscriptions","organizations_url":"https://api.github.com/users/dometto/orgs","repos_url":"https://api.github.com/users/dometto/repos","events_url":"https://api.github.com/users/dometto/events{/privacy}","received_events_url":"https://api.github.com/users/dometto/received_events","type":"User","site_admin":false},"created_at":"2015-12-28T15:21:10Z","updated_at":"2015-12-28T15:21:10Z","body":"@bartkamphorst, agree with your points. thanks again @AnvilStriker!"}}