Here is the code to bring Quicktags to Drupal 5.0. Archive contains whole Quicktags module based on latest cvs version.
- images moved to /images/ subfolder for consistency
- deprecated hook_settings replaced with menu callback
- .info file added
- add css using drupal_add_css()
... guess thats it basically ...

I didnt touch the javascript, but of course it can be updated to use jQuery!

CommentFileSizeAuthor
#14 quicktags5_3.patch4.96 KBprofix898
#10 quicktags_0.info113 bytesBèr Kessels
#8 quicktags5_2.patch4.07 KBprofix898
#7 quicktags5_1.patch2.93 KBprofix898
#6 quicktags5_js_api_0.patch4.64 KBprofix898
#3 quicktags5_0.patch2.97 KBprofix898
#2 quicktags5.patch2.64 KBprofix898
quicktags5.zip13.9 KBprofix898

Comments

Bèr Kessels’s picture

Bèr Kessels commented

Hello,

First of all, please don't post zip files. They make reviews a lot harder. check http://drupal.org/diffandpatch for more info.

Secondly, I will not branch to 5 until we have a way to release 4.7.x releases for contribs. Else we "loose" all the good and hard work gone into the 4.7-compatible HEAD version.

For ther rest, thanks a lot for the hard work. I will comment on the code(style) once i have a patch to look at.

profix898’s picture

profix898 commented
StatusFileSize
new quicktags5.patch2.64 KB

please don't post zip files

Ok. But AFAIK you cant move files in patches ;)

It was not hard work, but a quick run over the code ...
I was just needing a 5.0 compatible version of quicktags to play with hook_quicktags_insert in one of my own modules to port that one to 5.0. Its a little nasty if a module providing an api/hook (like quicktags) is not available for testing.

Here the patch (and remember I moved the images to /images/ folder) ...

profix898’s picture

profix898 commented
StatusFileSize
new quicktags5_0.patch2.97 KB

Accidently changed too much, here a corrected patch.

Bèr Kessels’s picture

Bèr Kessels commented

Hi,

I know you cannot move files in patches. But neither can you in a zip file. Hence the best option is to list what files need to:
* be moved
* be removed
* be added (and upload these files too).

And off course patches so I can review the changes.

Review:
* please adhere to Drupal coding standards;
} else { is wrong.
* you move images into /images, but leave the javascript and CSS in place, should we not move them into /styles/ and /javascripts/ too?
* 'description' => 'Configure Quicktags options.', must be t() ed. And mind capitalisation.

Bèr

Bèr Kessels’s picture

Bèr Kessels commented
Status: Active » Needs work
profix898’s picture

profix898 commented
Status: Needs work » Needs review
StatusFileSize
new quicktags5_js_api_0.patch4.64 KB

I didnt move js/css because its only one file each, but there were multiple images (to be grouped into a subfolder). The new patch also moves js/css as you suggested.
List of files to be moved:
- move all images 'ed_*.png' to '/images/' subfolder
- move 'quicktags.js' to '/javascript/' subfolder
- move 'quicktags.css' to '/styles/' subfolder

Your other issues (t() and else) are also corrected.
The patch still includes the 'hook to include javascript' stuff, but if you decide not to commit #88234, I can remove it. So its up to you ;)

profix898’s picture

profix898 commented
StatusFileSize
new quicktags5_1.patch2.93 KB

And a patch with only Drupal 5 stuff (just to be sure).

profix898’s picture

profix898 commented
StatusFileSize
new quicktags5_2.patch4.07 KB

Here is an update on my previous patch against HEAD (without moving images,... into seperate folders). It simply brings the latest quicktags.module (1.20) to Drupal 5 (incl. .info file.

Brent Rasmussen’s picture

Brent Rasmussen commented

I just installed this on my new 5.0 RC1 installation, and it works great! I am so glad to have my Quicktags back.

Thanks Thilo! It's working wonderfully well. Please add this to the list for the compatible 5.0 modules - at least on the head. Some of us regular, non-programmer types have a hard time applying patches. It's much easier to understand a complete archive containing the latest and the greatest that we can simply upload, then activate.

Thanks again,

-Brent

Bèr Kessels’s picture

Bèr Kessels commented
StatusFileSize
new quicktags_0.info113 bytes

merging issue 98932 into this one. Lets keep our actions synchronised and centralised please.

Bèr Kessels’s picture

Bèr Kessels commented

And here is Digital Spaghetti's patch. Being the last patch, does NOT mean its the best or the "one". My personal preference goes out to the patch in http://drupal.org/node/88200#comment-169510. I need some people to review both patches and possibly merge them into one final one.

Bèr Kessels’s picture

Bèr Kessels commented

http://drupal.org/node/98932 was marked duplicate of this issue.

profix898’s picture

profix898 commented

There's nothing to merge into #8. The patch already contains the .info patch from #10 (without package = quicktags, this shouldnt be used for single module packages). It also imports the stylesheet correctly (from #11).

profix898’s picture

profix898 commented
StatusFileSize
new quicktags5_3.patch4.96 KB

(I also suggest that we remove the ?q=quicktags code (see http://drupal.org/node/88234), what makes the menu callback redundant and also solves the javascript include issue (without any additional code/hook ;). This is not much more than code cleanup IMO. Modified patch attached.)

reikiman’s picture

reikiman commented

Subscribing. The users on the site I'm building really want this feature.

I haven't looked at the contents of any of the patches, but with multiple revisions of the patches I'm fearing I'll get confused trying to work out how to apply it. Is it possible to produce straightforward instructions? Or ??

profix898’s picture

profix898 commented

Take a look at http://drupal.org/node/60108 'Howto: Apply patches'. All the patches are against the latest version of Quicktags. What means you download latest Quicktags or get it from cvs and apply the patch. Then test it and post your opinion: Does it work? Any problems? Does the code look clean? ...

Bèr Kessels’s picture

Bèr Kessels commented

Note that the last part

Then test it and post your opinion: Does it work? Any problems? Does the code look clean? ...

is also in your own benefit: It means that with positive reviews I can commit the patch , and you can start running the stable version on your site sooner.

reikiman’s picture

reikiman commented

I know how to use the patch program .. I've been using it since the 1980's. What I was asking is which of the five or so patches on this issue do I use? And some of the comments talk about moving files around, and I don't see it clearly described what files to move where, etc.

profix898’s picture

profix898 commented

Try the patches in #8 and #14 in the first place. #8 is a simple port of the latest Quicktags (without any files being moved) and #14 additionally makes a small improvment to the code that adds the javascript.

reikiman’s picture

reikiman commented

I just applied the patch in comment #14. The test site is running rc1.

All the buttons worked and inserted stuff appropriately. I'm happy with the behavior of each of the buttons.

The <!--break--> when inserted shows on the screen as <!--break--> ... don't you mean that to be the signal to Drupal for a break on the teaser? The node type is blog-entry.

Standart’s picture

Standart commented

Patch #14 works for me running Drupal 5.0.

I guess the <!--break--> issue has been fixed in 5-RC2 and of course 5.0.

Bèr Kessels’s picture

Bèr Kessels commented

It was not so much broken, and therefore, is not 'fixed' now either. Problem is that core decided to introduce a hardcoded solution for teasers. We must make sure quicktags at least works toghether with that solution.

If that is not possible, then we must develop something to unset the core-behaviour, and if possible, create a solution in quicktags that emulates that core behaviour.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs review » Reviewed & tested by the community

Patch works for me. I think its good enough to commit and move on.

Bèr Kessels’s picture

Bèr Kessels commented
Status: Reviewed & tested by the community » Needs review

neclimdul: what patch did you test?

analytik’s picture

analytik commented
Title: Quicktags for Drupal 5.0 » Quicktags for Drupal 5.0 - yet another version

To add to the confusion, without knowing about this thread, I've created my own version of quicktags, without the fugly icons.
http://drupal.org/node/113146

analytik’s picture

analytik commented
Title: Quicktags for Drupal 5.0 - yet another version » Quicktags for Drupal 5.0

whoops, sorry for the accidental rename.

neclimdul’s picture

neclimdul
He/Him
commented

Was reviewing:
http://drupal.org/node/88200#comment-169703

Looking at it again I'd kinda like the menu array to be indented to the coding standards but the code works just fine.

denney’s picture

denney commented

Tested code in #14 and it works fine.

Subscribing so I know when the changes get commited.

Bèr Kessels’s picture

Bèr Kessels commented
Status: Needs review » Closed (duplicate)

I tried to exoplain before, that I am not happy with the fact that this patch introduces new features (i.e. add the JS to the HTML body). Not that that is not a good feature, I like it, but it keeps me from the REAL purpose of this patch, to upgrade to 5.x.

Therefore, I a closing this thread. It is fixed in dedicated threads:
http://drupal.org/node/115580
http://drupal.org/node/115583
http://drupal.org/node/115585

And one issue is still open in thread:
http://drupal.org/node/115586