Improve Sitemap method-missing implementation#473
Merged
n-rodriguez merged 5 commits intokjvarga:masterfrom Mar 20, 2026
Merged
Improve Sitemap method-missing implementation#473n-rodriguez merged 5 commits intokjvarga:masterfrom
n-rodriguez merged 5 commits intokjvarga:masterfrom
Conversation
jasonkarns
force-pushed
the
method-missing-proper
branch
from
f2bd91b to
b9f92e4
Compare
jasonkarns
force-pushed
the
method-missing-proper
branch
2 times, most recently
from
7deb532 to
4d16352
Compare
When the LinkSet doesn't respond to a particular method, method-missing should defer to its ancestors. Ideally, only public methods are delegated to LinkSet. (And it should only be using `public_send`) But evidently it currently assumes delegation of even private methods 😱 so preserve that behavior. But as long as we're using private send, we should at least be using the proper method: __send__
It is not user-friendly to have anonymous classes. They are hard to debug. So let's give the SitemapGenerator::Sitemap's class a name (Config)
jasonkarns
force-pushed
the
method-missing-proper
branch
from
4d16352 to
c0f43bd
Compare
jasonkarns
commented
Mar 16, 2026
| Sitemap = (Class.new do | ||
| def method_missing(*args, &block) | ||
| (@link_set ||= reset!).send(*args, &block) | ||
| Sitemap = (Config = Class.new do |
Author
There was a problem hiding this comment.
Giving the anonymous class a name here to enhance debug-ability. Looking at classes in the inheritance hierarchy without names is difficult to trace and understand.
Open to alternatives besides SitemapGenerator::Config, though.
jasonkarns
added a commit
to jasonkarns/sitemap_generator
that referenced
this pull request
Mar 21, 2026
* upstream/master: Enhance Rails railtie to respect existing rails configuration (kjvarga#478) Improve Sitemap method-missing implementation (kjvarga#473) Fix broken specs (kjvarga#476) Simplify rake task hierarchy (kjvarga#477) Fix AWS upload deprecation (kjvarga#464)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix up the implementation of method-missing for the SitemapGenerator::Sitemap parent class.
method missing implementations should:
respond_to_missing?, notrespond_to?Also
__send__, notsendonly delegate public methods so as to not break encapsulation and leak private methodsFixes #472