? Success

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000 okonomiyaki3000 - open - 26 Jun 2014
avatar okonomiyaki3000 okonomiyaki3000 - change - 26 Jun 2014
Title
CodeMirror 4
[#31822] CodeMirror 4
avatar roland-d
roland-d - comment - 26 Jun 2014

@test: The PR now applies but with some issues:

D:\wamp\www\joomla-cms>curl https://github.com/joomla/joomla-cms/pull/3833.diff | git apply
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 2972k 100 2972k 0 0 1441k 0 0:00:02 0:00:02 --:--:-- 1546k
:37062: trailing whitespace.

:37166: trailing whitespace.
*
:37176: trailing whitespace.

  • Collection classes mixing in this trait provide a concrete :37181: trailing whitespace. * :37184: trailing whitespace. * warning: squelched 167 whitespace errors warning: 172 lines add whitespace errors.

I guess these are issues in the Codemirror files rather than Joomla! code.

Other than that all looks good for me. The Fira Mono font looks pretty nice too.

avatar Bakual
Bakual - comment - 26 Jun 2014

There are changes in files unrelated to Codemirror. So please remove syntax="html" and the other unrelated changes from the following files so this PR only affects files regarding codemirror itself.

  • administrator/components/com_categories/models/forms/category.xml
  • administrator/components/com_content/models/forms/article.xml
  • administrator/components/com_templates/views/template/tmpl/default.php
avatar okonomiyaki3000
okonomiyaki3000 - comment - 27 Jun 2014

I guess syntax="html" is not actually necessary since it's the default anyway.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 27 Jun 2014

The changes to administrator/components/com_templates/views/template/tmpl/default.php are related to CM though. A message about how to use CM is written in to the template file for some reason instead of being displayed by CM. We need to remove this message because CM will display it now.

avatar Bakual
Bakual - comment - 27 Jun 2014

Ah I see :+1:

avatar infograf768
infograf768 - comment - 27 Jun 2014

The xml description string change is unecessary.
PLG_CODEMIRROR_XML_DESCRIPTION="This plugin loads the CodeMirror (v4.3) editor."

let's keep the former one:
PLG_CODEMIRROR_XML_DESCRIPTION="This plugin loads the CodeMirror editor."

The new version is stated in codemirror.xml.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 27 Jun 2014

I don't mind changing that back (it makes updates slightly less troublesome anyway) but then I wonder if we should display xml version information in more places. Now it's only displayed in the Extensions Manager but maybe displaying in the Plugin Manger would be useful. There's certainly enough room for another column in the list. I suppose that's a task for a different PR.

avatar infograf768
infograf768 - comment - 28 Jun 2014

If we do that for plugins, then someone will ask to do it for modules where the manager is crowded. I guess we can live with the version being displayed in Extensions Manager=>Manage. :)

avatar okonomiyaki3000
okonomiyaki3000 - comment - 23 Jul 2014

Looks like not many important changes in 4.4 but I don't want to fall behind so here it is.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Jul 2014

Well, I hope it's pretty self-explanatory but, after applying the patch, you should modify the plugin options because they've changed a lot. You'll find many new options for the appearance of the editor, font choices, color schemes, etc. Then, use the editor to edit any kind of code, it should do proper syntax highlighting for html, css, javascript, php, etc.

avatar Bakual Bakual - reference | ef8c75a - 27 Jul 14
avatar Bakual
Bakual - comment - 27 Jul 2014

Merged into 3.4-dev branch. Thanks!

avatar Bakual Bakual - close - 27 Jul 2014
avatar Bakual Bakual - change - 27 Jul 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-07-27 15:43:00
avatar Bakual Bakual - close - 27 Jul 2014
avatar okonomiyaki3000 okonomiyaki3000 - head_ref_deleted - 28 Jul 2014

Add a Comment

Login with GitHub to post a comment