In commit 84644 conflict detection for geshifilter and codefilter was added. On my site I have input formats where both codefilter and geshifilter are enabled but where geshifilter is NOT set to handle the code tag, and therefore I don't think there is actually a conflict. Nonetheless, when I go to my administration page, I'm given a urgent looking warning about geshifilter needing to have conflicts resolved.

So, I set about trying to make the conflict detection of geshifilter a bit smarter.

I'm setting this patch to code needs work only because as is this patch doesn't actually work when applied to geshifilter. I have to comment out 3 lines in the function _geshifilter_get_tags() (that issue is at http://drupal.org/node/184996).

So, if #184996 is really a bug and gets fixed, then the status of this patch should be CNR. But if #184996 is not a bug, I may need to work on this patch more.

Comments

aclight’s picture

Status: Needs work » Needs review

Now that http://drupal.org/node/184996 is fixed, this patch should actually work properly.

soxofaan’s picture

On my site I have input formats where both codefilter and geshifilter are enabled but where geshifilter is NOT set to handle the code tag

I am interested why you use codefilter and GeSHi filter in the same input format.
What does codefilter offer that is not possible with GeSHi filter? Is it the support for <?php ?> snippets?

aclight’s picture

I am interested why you use codefilter and GeSHi filter in the same input format.
What does codefilter offer that is not possible with GeSHi filter? Is it the support for snippets?

I've been using both codefilter and geshifilter on my site in the same input format for a while now, and I don't see the need to remove codefilter (the inertia principle :). As you indicated, I do like support for <?php ?> tags. Plus, until fairly recently (when you apparently took over geshifilter) it looked like geshifilter wasn't being maintained at all and since codefilter is used on d.o I wasn't worried about it being maintained, so I wanted codefilter to handle everything that it could and then just have geshifilter handle additional languages.

Now that geshifilter has been improved a lot lately this is less of a concern, but I think I still want codefilter to handle what it can and use geshifilter for the rest.

But separate from what I'm doing on my site, I don't think it makes sense for geshifilter to put up a warning to disable one of the filters when there isn't actually any potential for a conflict (which, to my understanding, is the case when geshifilter is not set to handle the code tag). I know it's easier just to put up a warning if both filters are enabled for an input type, so that's why I wrote the patch.

soxofaan’s picture

I think there is little point in making it possible to enable codefilter and GeSHi filter in the same input format, since I see GeSHi filter as a more powerfull codefilter, which should be able to act as a drop-in replacement (and not just as a supplement).

I am planning to add support for < ?php .. ? > tags. When that is done, are there (for you) other reasons to let codefilter and GeSHi filter coexist in the same input format? Beside the < ?php .. ? > tags, I don't see more codefilter stuff that GeSHi filter can't do.

BTW: I think that you forgot to attach the patch, I can't find it in this thread.

aclight’s picture

StatusFileSize
new3.46 KB

Whoops...here's the patch.

aclight’s picture

@soxofaan

I am planning to add support for < ?php .. ? > tags. When that is done, are there (for you) other reasons to let codefilter and GeSHi filter coexist in the same input format? Beside the < ?php .. ? > tags, I don't see more codefilter stuff that GeSHi filter can't do.

I know that in the past the HTML code output by GeSHi was very inefficient since the styles were inline instead of in a .css file. I see that you've recently added an option to handle the GeSHi option to not put the styles inline, but I haven't looked into how this works yet. I have CSS preprocessing enabled on my site--would enabling CSS styles with GeSHi have any problem working with CSS preprocessing turned on?

If you added < ?php ? > tag support in geshifilter than I can't think of any features that codefilter would have that geshifilter doesn't. Whether I would stop enabling the two filters in the same input format, I don't know. What are your plans for writing geshifilter for Drupal 6?

soxofaan’s picture

here's the patch.

Well, I introduced the conflict detection stuff. The old GeSHi filter automatically tried to resolve the conflicts by poking in the settings of other modules, which seemed a bit dangerous to me. As a user I think it's nice to have this type of functionality but as developer/maintainer I don't like the code behind it. It's messy, deeply nested, hard to refactor/generalize, etc. So I don't like it to grow.
You patch makes sense, but I'll have to look if I could do the whole conflict detection stuff in a more elegant way.

I have CSS preprocessing enabled on my site--would enabling CSS styles with GeSHi have any problem working with CSS preprocessing turned on?

GeSHi filter uses the standard way with drupal_add_css() to add the CSS style sheet, so it is compatible with CSS preprocessing.
The only problem I found is that the CSS stylesheet is dynamicaly generated when languages are enabled/disabled in your setup. So if you enable languages after CSS preprocessing is enabled, those languages will not get CSS rules in the CSS aggregate. A simple workaround is disabling and enabling CSS preprocessing.

What are your plans for writing geshifilter for Drupal 6?

I didn't use D6 yet, but I'll start looking into it when the RC's come out.

soxofaan’s picture

Version: 5.x-2.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

fixed for 5.x-2.x-dev by http://drupal.org/cvs?commit=98566

soxofaan’s picture

Status: Patch (to be ported) » Fixed
Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.