incubator-brooklyn icon indicating copy to clipboard operation
incubator-brooklyn copied to clipboard

[BROOKLYN-166] add codemirror, yaml syntax-highlighting, autocompletion)

Open azbarcea opened this issue 9 years ago • 37 comments

I was looking into the codemirror integration.

I didn't integrate it in all the views, only in application-add-wizard.js, and I look forward to your feedback.

I tried to improve the scroll down and now, there aren't 2 scrollers. There is only one for the editor.

The following things are still open:

  • I excluded rat-plugin for the files added (actually, right now excludes the whole src).
  • I had to modify the original codemirror addons and modes to include codemirror instead of ../../lib/codemirror
  • The way angular.js views and require.js is used (using the config.js), make the presumptions the javascript libraries are static in libs. Similar to mvn, I propose grunt and npm to be used, so that the whole libs directory to disappear.
  • I have no yaml-hint.js, or even camp-hint.js. I need to study further the CAMP spec for it. I think also that brooklyn has a little different spec for .yaml files, extending the language. I am right? Right now, I used the anyword-hint.js, which adds to the hints (auto-completion), the words that already exist in the document.
  • There are many themes that can be added, I will also add a configuration for: addons, modes, themes to be used globally. Maybe this should be at user profile level ?!?

azbarcea avatar Aug 23 '15 22:08 azbarcea

Testing from usage/dist/target/brooklyn-dist/brooklyn/bin/brooklyn launch doesn't work.

Tested with

org.apache.brooklyn.rest.jsgui.BrooklynJavascriptGuiLauncher -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005

I'll take a look at what's missing, tomorrow

azbarcea avatar Aug 23 '15 23:08 azbarcea

incubator-brooklyn-pull-requests #1728 SUCCESS This pull request looks good

asfbot avatar Aug 23 '15 23:08 asfbot

Tested, it works great. There are indeed two scrolls but that should be easily fixable with a bit of CSS. Also, the editor is only one line height, could we extend to the height of the pop-up by default?

Regarding the autocompletion, my view on that is first to pull out the catalogue entities when we are on a type: line. We can even imagine to have specific hints based on where we are, like having all the provisioning properties after a provisioning.properties: and so on.

tbouron avatar Aug 26 '15 07:08 tbouron

@tbouron, regarding extending to the hight of the pop-up, we could integrate the tree view you proposed a while back. I don't know if that could be done without enlarging the pop-up which will impact the other tabs. Unless we make it a different pop-up or view. WDYT?

hzbarcea avatar Aug 26 '15 11:08 hzbarcea

A few comments:

  • If I have nothing in the catalog, it shows me YAML editor and it looks nice. Surprising at first how lines are added but it's nice. Cut and paste and other things work as expected.
  • If I then click on the "catalog" tab, with nothing there, i have option to write YAML blueprint. I click that button and it returns me to the YAML tab, but a new editor is added. This looks like a bug.
  • If I leave the wizard, go to the catalog and add some YAML (e.g. default.catalog.bom from CLI project), then open the add wizard, I have the catalog tab selected (good), but then when I go to the YAML editor I get the old YAML view.

Of course we need RAT re-enabled.

Code completion will require some REST API enhancements or better understanding of the model in the client. I'd also like to see this as a separate main tab, rather than a wizard. But we can leave these for a follow-on PR, as with the bug fixes before this paragraph resolved, this is a nice improvement.

ahgittin avatar Aug 26 '15 14:08 ahgittin

@ahgittin - I will take a look to the bug. Also keep in mind that is not integrated in all the views, neither the events aren't propagated.

@tbouron, is there a documentation for the CAMP syntax/semantics (for hint) besides the spec?

azbarcea avatar Aug 26 '15 18:08 azbarcea

There is a Brooklyn camp yaml reference on the website. Most useful thin would be autocomplete for catalog item types and their configuration keys both of which are exposed through rest api. On 26 Aug 2015 19:15, "Alexandru Zbarcea" [email protected] wrote:

@ahgittin https://github.com/ahgittin - I will take a look to the bug. Also keep in mind that is not integrated in all the views, neither the events aren't propagated.

@tbouron https://github.com/tbouron, is there a documentation for the CAMP syntax/semantics (for hint) besides the spec http://docs.oasis-open.org/camp/camp-spec/v1.1/camp-spec-v1.1.html?

— Reply to this email directly or view it on GitHub https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135128315 .

Cloudsoft Corporation Limited, Registered in Scotland No: SC349230. Registered Office: 13 Dryden Place, Edinburgh, EH9 1RP

This e-mail message is confidential and for use by the addressee only. If the message is received by anyone other than the addressee, please return the message to the sender by replying to it and then delete the message from your computer. Internet e-mails are not necessarily secure. Cloudsoft Corporation Limited does not accept responsibility for changes made to this message after it was sent.

Whilst all reasonable care has been taken to avoid the transmission of viruses, it is the responsibility of the recipient to ensure that the onward transmission, opening or use of this message and any attachments will not adversely affect its systems or data. No responsibility is accepted by Cloudsoft Corporation Limited in this regard and the recipient should carry out such virus and other checks as it considers appropriate.

ahgittin avatar Aug 26 '15 19:08 ahgittin

I think it's this page, right?

https://brooklyn.incubator.apache.org/v/latest/yaml/yaml-reference.html

CMoH avatar Aug 27 '15 08:08 CMoH

Yes @CMoH, this is the page that @hzbarcea was looking for.

tbouron avatar Aug 27 '15 08:08 tbouron

@hzbarcea To be honest, I'm not so sure anymore about the drag and drop editor. It is certainly a good feature for a sales point of view but I'm wondering about the usability. I think that having a good YAML editor with a good autocompletion feature over Brooklyn CAMP spec is a better approach for now.

We could also imagine to have something a bit like swagger, i.e. having a live representation of the YAML as a tree or nodes. That would be great!

tbouron avatar Aug 27 '15 08:08 tbouron

I think a drag-and-drop editor is very important for new users, especially if it generates the YAML which will help them move to become advanced users.

ahgittin avatar Sep 08 '15 13:09 ahgittin

@azbarcea can you address the issues blocking this being merged?

ahgittin avatar Sep 08 '15 13:09 ahgittin

ping @azbarcea

ahgittin avatar Sep 15 '15 16:09 ahgittin

incubator-brooklyn-pull-requests #1815 FAILURE Looks like there's a problem with this pull request

asfbot avatar Sep 18 '15 00:09 asfbot

@ahgittin, @CMoH - fixed CodeMirror integration.

Please review. I look forward to your feedback.

azbarcea avatar Sep 18 '15 00:09 azbarcea

fixed merge ... with latest code base

azbarcea avatar Sep 18 '15 00:09 azbarcea

incubator-brooklyn-pull-requests #1816 SUCCESS This pull request looks good

asfbot avatar Sep 18 '15 00:09 asfbot

I fixed the indentation ... any thoughts?

azbarcea avatar Sep 18 '15 21:09 azbarcea

incubator-brooklyn-pull-requests #1822 SUCCESS This pull request looks good

asfbot avatar Sep 18 '15 21:09 asfbot

LGTM

tbouron avatar Sep 21 '15 10:09 tbouron

  • bin/brooklyn launch doesn't work
  • RAT is still disabled (**//src/*)
  • Re-opening the dialog shows the old textarea

neykov avatar Oct 05 '15 10:10 neykov

Also move lib/codemirror.js, addon/*, mode/* to libs/codemirror folder.

neykov avatar Oct 05 '15 12:10 neykov

I cannot move lib/codemirror.js,addon/* to libs/codemirror folder, see explanation above ...

azbarcea avatar Oct 07 '15 11:10 azbarcea

bin/brooklyn launch works - perhaps there was some external reason it didn't on you machine, @neykov

CMoH avatar Oct 07 '15 12:10 CMoH

also note that the catalog yaml syntax is different, as described at https://brooklyn.incubator.apache.org/v/latest/ops/catalog/index.html . it allows you to paste a simple blueprint or several such. not a blocker for this but important when completion is enabled; maybe for now a quick win would be to have a sample template with the right syntax, or comments showing the right syntax.

i confirm bin/brooklyn launch works nicely. and switching between tabs in the "add application" wizard causes no problems (but it isn't using the new codemirror any more.)

newly found issues:

  • check/fix overflow-x: scroll for other textareas
  • add license info for codemirror in overrides.yaml
  • the JS console gives an error trying to load /assets/css/complete.css

others still outstanding:

  • if possible, put 3rd party deps into libs/ like the others, but if that's too hard leave as is
  • the rat check needs to scan all files or have an explanation. if it's really really hard to have the codemirror files live under libs/ then call them out explicitly here with a comment explaining why they have the unusual structure
  • update the two TODO comments, or expand on who/when they'll get done

if we could add this to the wizard -- or completely rewrite the wizard as part of drag-and-drop -- then that would be good, but not needed in this PR.

i think very soon we should start on a new UI based on angular and a npm/grunt/bower toolchain and with proper tests. the tooling has come on leaps and bounds since this UI was begun! also important that both the UI and the REST API be extensible. /cc @morganbrooke

ahgittin avatar Oct 08 '15 15:10 ahgittin

@azbarcea what's the status of this?

the latest changes address the licensing and rat issues, however still outstanding are:

  • check/fix overflow-x: scroll for other textareas
  • error in JS at rungime trying to load /assets/css/complete.css
  • 3rd party deps into libs/ like the others (if not too hard, else comment)
  • provide more info where you have added TODO comments

plus new issue:

  • revert auto-scanning of classpath for REST API; it significantly increases startup time and typically we don't want it (you can trigger a scan by POSTing a suitable catalog.bom if you need to, or add those explicit items you wish to have); if you think it should be there, please give more details

ahgittin avatar Oct 28 '15 13:10 ahgittin

I am addressing all the issues above ...

azbarcea avatar Oct 29 '15 23:10 azbarcea

@azbarcea many of those issues don't look like they've been addressed. what's the latest?

ahgittin avatar Nov 04 '15 15:11 ahgittin

Having issues with the current Modal Wizzard and adding the CodeMirror editor, I'm proposing having the Editor as a tab next to Home.

apachebrooklyn-editor-blueprint

apachebrooklyn-editor-detail

The proposal and the updates will continue on this PR.

azbarcea avatar Nov 18 '15 13:11 azbarcea

@azbarcea +1. maybe call it "Composer" rather than "Editor", and I think don't show the catalog on this tab (we have the catalog tab).

ahgittin avatar Nov 18 '15 15:11 ahgittin