I'm -1 on this change. Not only is it backwards incompatible and slows down loading of lexers for most users, it also allows any installed package to override core lexers, which is a maintenance nightmare if they declare wrong/overly broad patterns.

I'm not convinced we should keep the current plugin system in general. Am explicit API to register new lexers (which could then override built-in ones) would cover most use cases, I believe.

I believe that there needs to either be a way to override the built-in lexers, styles, formatters, and filters or at least a way to explicitly disable a built-in so that a plugin can be used instead. Perhaps a command line argument could be added instead to allow users to specify what they want to disable from the built-ins. That would make it so that users have to explicitly disable them rather than relying on a plugin to override them (so that it is more clear and less prone to problems with misbehaving plugins). It would also no longer affect load time, since it would act the same as master does currently.

Not that I know much about this topic, so sorry if this is irrelevant, but doesn't this have a bit of a security risk? A package supposedly installed for unrelated purposes (which one might be running in a sandboxed environment normally) can silently start executing code in Sphinx documentation or other operation of Pygments (which one might trust enough to run not sandboxed). Currently, it won't happen without something explicit in the Sphinx configuration (like specifying altpython if your lexer is for Python and defines altpython as alias to be able to override the built-in lexer).

I don't think it would be a security issue if I change it to have a command line argument to explicitly disable a built-in lexer/style/formatter/filter rather than making it so that plugins can implicitly override (as suggested in my above comment). This suggested change would revert my current proposed changes and instead add a command line argument which would need to be explicitly passed in. This should alleviate any security concern because the call to pygments would need to have this argument before any override could take place. I believe this also alleviates @birkenfeld 's concern about backwards compatibility because there would be no change for users using the existing command line arguments.

Yes, I'm fine with it if it's explicitly requested. This command line switch would also be exposed as a flag in the API, I assume?

Yeah, I would think it would make sense as an explicit option in all forms of running pygments (command line, api, etc.)

I have removed the implicit plugin override changes and replaced them with explicit options for disabling lexers/styles/filters/formatters. Hopefully this alleviates concerns from @birkenfeld and @jean-abou-samra .

Is there anything else I should do for this pull request before it is eligible for review? Sorry I haven't contributed to this project before and I didn't see anything specific in the contribution guidelines.

Sorry, no, I think you're good. We just need some cycles to get to it. I added the 2.12 milestone now to ensure you're not forgotten.

Looking at the CI, a rebase should fix the error there.

Looking at the CI, a rebase should fix the error there.

I pulled the latest changes from master, but I'm not sure what you are suggesting to do with a rebase. Could you please elaborate?

You can also merge with master, but for smaller PRs it's usually preferable to rebase on top of master, since it avoids the commit history getting a tangled mess :)

In any case, your branch is missing f9c7fb6 which fixes the tests failing due to a pytest update.

Gotcha. I'll look into rebasing instead of doing a merge tonight. Looking at it some more, there's a few changes I'd like to make as well to allow disabling to take effect for the -L and -H command line arguments, so I'll look into that tonight and push an update to this PR when I'm done.

Great! I was wondering about tests, but not sure how easily this can be tested. If you can think about something, give it a try :)

I've done the rebase as well as added more changes to allow for the disabled builtins to apply to other command line arguments. I'll take a look into writing tests over the weekend.

I added a few simple tests for command line functionality. Let me know if you think more tests for specific functionality should be added or if anything else should be changed.

I'm missing some documentation for this -- I thought the command line option would be explained somewhere? Am I missing something here? I think it should also get explained in the plugins documentation.

I'm missing some documentation for this -- I thought the command line option would be explained somewhere? Am I missing something here? I think it should also get explained in the plugins documentation.

I just added some documentation. Please let me know if you think any of the documentation needs elaborated more on.

I think this is good to go now. Any concerns, @birkenfeld or @jean-abou-samra ? I'd like to merge this for the next release.

I think this is good to go now. Any concerns, @birkenfeld or @jean-abou-samra ? I'd like to merge this for the next release.

Well, I'm mildly OK with this, but I can't say I love it. Part of it is already existing issues with setuptools entry points. If one plugin is non-functional, it will cause an error and prevent others from being used. If several plugins are providing support for the same language, they cannot be used with the same Python environment. New lexers must be registered via plugins, pulling in the whole packaging machinery. If we want a better way to register plugins, I'd rather see an API looking like

from pygments.lexer import register_lexer
from myplugin import TheLexer

register_lexer(TheLexer)

and corresponding command-line flags. I wanted to prepare a PR for this as an alternative to this PR at some point, but I don't have the time at the moment.

The question for me is, do we want to merge this in the meantime or do it "properly" for a future release? This is a relative large change, and honestly I haven't looked much at plugins in Pygments and the possible problems with them. The change here seemed "ok-ish" in terms of impact, and I'm mildly concerned to not have a solution for a long time if we don't merge it :(

Please give me time to review this, currently life is in the way...

@jean-abou-samra Any update on this? I haven't gotten a response about my question posted on March 27th.

Sorry for the delay, I had to take some time off Pygments. I've been working on a new plugin system which should address this; the patch is nearing completion, and I've opened #2152 as a preliminary.

I've been working on a new plugin system which should address this; the patch is nearing completion, and I've opened #2152 as a preliminary.

Do you have a branch or fork for this new plugin system?

I am a little hesitant to share WIP code, but I guess #1603 (comment) should give you a good idea.