Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In stead of manually installing the Javascript library, why not fetch and install it through composer.json on installation? This guarantees that the library is always present, the correct version and in the correct location.
Will try to provide a patch
Comment | File | Size | Author |
---|---|---|---|
#29 | 2931474-29.patch | 1.44 KB | voleger |
Comments
Comment #2
b.lammers CreditAttribution: b.lammers commentedCreated an addition for the README.txt to explain how to install the JavaScript library using Composer. Adding it to the module composer.json did not work (up to this point)
Comment #3
b.lammers CreditAttribution: b.lammers commentedComment #4
kevinquillen CreditAttribution: kevinquillen at Velir commentedI don't know if this can work automatically, generally from this point right here:
Your root is /web, whereas mine is /docroot.
Comment #5
b.lammers CreditAttribution: b.lammers commentedI understand, put it in as an example, if the user adds lines to their composer.json manually I assumed the knowledge would be there to adapt it to the situation.
New patch added with a bit more obvious example of what needs to be replaced.
Comment #6
cashwilliams CreditAttribution: cashwilliams at Acquia commentedOne more nitpick - I think it should make it clear it goes in the repositories section, and explain adding a repositories section if one doesn't exist.
Comment #7
KarimB CreditAttribution: KarimB as a volunteer commentedHere is the patch based on #5. (Nice job @b.lammers!)
I added the whole repositories section, so it's more clear where to place the code.
I also deleted the line with ""codesnippet": " in the repositories section because the patch doesn't work with this line.
Best regards.
Comment #8
xmacinfoThe drupal-composer project (https://github.com/drupal-composer/drupal-project/blob/8.x/composer.json) uses :
composer/installers
should we change the instructions to use that one?
The rest of this patch is fine and tested using "composer/installers".
Comment #9
KarimB CreditAttribution: KarimB as a volunteer commentedYes @xmacinfo, you're right. It should be "composer/installers" with a "s".
Here is the patch with the correction.
Best regards.
Comment #10
hugronaphor CreditAttribution: hugronaphor at Acrosto for manifesto commentedThe patch worked well for me as a guide to quickly set up the library but
do we really need to specify the version in:
5. Run 'composer require ckeditor/codesnippet:4.8.0' add the library itself to your composer required list.
I think this is not needed as the version is defined strictly above as part of the repository.
Comment #11
xmacinfoPlease have a look at :
https://ckeditor.com/cke4/addon/codesnippet
Codesnippet versions relate to CKEditor version. I believe 4.8.0 is the safest version for Drupal 8 CKEditor. But I did not test newer versions.
Comment #12
hugronaphor CreditAttribution: hugronaphor at Acrosto for manifesto commentedNo, I mean: we define the repository with
"version": "4.8.0",
And if I do
composer require ckeditor/codesnippet
there is no way I can get anything other than the version defined above. Am I right?Comment #13
xmacinfoIf you do
composer require ckeditor/codesnippet
the composer.json file will be rewritten and it won't, probably, use 4.8.0.The problem here as that we use an external package. In the composer file we look specifically for the 4.8.0 ZIP file.
Loading external files not supported by Packagist can be tricky.
Comment #14
xmacinfoLook for :
"url": "https://download.ckeditor.com/codesnippet/releases/codesnippet_4.8.0.zip",
…And replace the version number.
Comment #15
KarimB CreditAttribution: KarimB as a volunteer commentedYou're absolutely right @hugronaphor !
Here is the patch without indicating the version in the composer require command.
Thx !
Comment #16
KarimB CreditAttribution: KarimB as a volunteer commentedSorry, in the last the patch wasn't with the correct message number.
Now the correct one.
;)
Comment #17
KarimB CreditAttribution: KarimB as a volunteer commentedHere is the output code without specifying the version in the composer command
composer require ckeditor/codesnippet
and it looks that it's downloading the 4.8 version of the library as expected.Comment #18
kiwimind CreditAttribution: kiwimind commentedShould be `composer require composer/installers`.
Otherwise this seems fine, am currently using patch (with incorrect text).
Comment #19
kiwimind CreditAttribution: kiwimind commentedComment #20
hugronaphor CreditAttribution: hugronaphor commentedAddressed 18th comment
Comment #21
kiwimind CreditAttribution: kiwimind commentedPatch applied and instructions tested. Looks good to me.
Seems like an important addition too, as this module could be deemed unusable with some deployment processes.
Thank you.
Comment #22
kevinquillen CreditAttribution: kevinquillen at Velir commentedInstructions look good, just tried it and worked well. Sorry for the delay on this.
Anyone want to help out with maintaining?
Comment #23
anavarrePatch in #20 didn't apply for me.
And
Here's a new patch that applies correctly, but there's no interdiff.
codesnippet
to mimic what we already do e.g. fordrupal
(d.o. composer façade) orassets
(Assets Packagist)P.S.: too bad there's no Bower or NPM Code Snippet package available. This would be way more easier to require the library.
Comment #24
Waliur CreditAttribution: Waliur commented#23 worked for me. Thanks anavarre!!!
Comment #25
AkashKumar07 CreditAttribution: AkashKumar07 commented#23 worked for me too. Moving to RTBC.
Thanks @anavarre
Comment #26
sinasalek CreditAttribution: sinasalek as a volunteer and commentedHere is the more maintainable way of doing this :
https://www.drupal.org/docs/8/modules/webform/webform-frequently-asked-q...
Comment #27
sinasalek CreditAttribution: sinasalek as a volunteer and commentedI created a patch for a more uptodate way and easier way
I'm using Drupal recommend composer template and the following patch just works
composer require drupal/codesnippet and that's it
Comment #28
sinasalek CreditAttribution: sinasalek as a volunteer and commentedhere is the patch
Comment #29
volegerLinking similar discussions.
Here the patch with the dependency on
drupal-ckeditor-libraries-group/codesnippet
package.So from the developer side there no needs to provide additional instructions for install via Composer.
You can try how the approach works on the simplytest.me with module ckeditor_font https://simplytest.me/project/ckeditor_font/8.x-1.x
Comment #30
WebbehGiven the comment in #23:
> P.S.: too bad there's no Bower or NPM Code Snippet package available. This would be way more easier to require the library.
Coupled with the work in #29, marking this as Needs Review for this proposed direction.
Comment #31
jigariusI tried the patch in #29. It involved 2 steps.
First, apply the patch to the module using the "patches" section of the composer.json file.
Second, require the libraries using composer: "composer require drupal-ckeditor-libraries-group/codesnippet:4.14.0"
If this is the expected workflow, then it works. I was thinking that the libraries would be installed with the module itself.
Comment #32
volegerThe patch wouldn't make any effect on the module in the drupal project. Composer working with the metapackage information before applying the patches. So no-action-required approach would be available when the changes would be done in the codesnippet module repository. In that way drupal.org's packagist will tell for your project composer also to download required module dependency.
Check it out the ckeditor_font module which already uses that approach. Composer resolves library dependency.
Comment #33
jigariusCool. So, it works. After the patch has been committed, the library should automatically be installed.
Comment #34
solideogloria CreditAttribution: solideogloria commentedYeah, the patch has to be committed in order for it to be installed automatically.
Comment #35
jcnventura CreditAttribution: jcnventura at 1xINTERNET commentedThanks for the patch, @voleger!
Comment #37
jcnventura CreditAttribution: jcnventura at 1xINTERNET commentedChanged the patch a bit to cope with evolving library versions, and also to state the documented minimum (4.5.11). On my tests, it nevertheless installed 4.14.0.