Skip to content

[TASK] Drop getLineNo() from the Renderable interface #1038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

oliverklee
Copy link
Collaborator

The concept of being able to get rendered is logically independent from having a position in the input stream.

Hence, both concepts should not be intermingled in the Renderable interface.

(We might want to add a dedicated interface for classes that are linked to specific points in the input stream later, though.)

@coveralls
Copy link

coveralls commented Feb 28, 2025

Coverage Status

coverage: 55.282%. remained the same
when pulling 46b15d2 on task/drop-lineno-from-renderable
into 19ffd07 on main.

The concept of being able to get rendered is logically independent
from having a position in the input stream.

Hence, both concepts should not be intermingled in the
`Renderable` interface.

(We might want to add a dedicated interface for classes that
are linked to specific points in the input stream later, though.)
@oliverklee oliverklee force-pushed the task/drop-lineno-from-renderable branch from 1da74d1 to 46b15d2 Compare February 28, 2025 19:52
Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No deprecation notice required, since classes implementing the original interface will still have the method. (And PHP is not strict about whether a method is defined in the parameter type expected, as long as it exists in the actual class of object provided.)

@JakeQZ JakeQZ merged commit 2093b19 into main Feb 28, 2025
21 checks passed
@JakeQZ JakeQZ deleted the task/drop-lineno-from-renderable branch February 28, 2025 23:29
@JakeQZ
Copy link
Collaborator

JakeQZ commented Feb 28, 2025

intermingled

I changed this in the commit message to "conflated", which is possibly more accurate, though "intermingled" was fine.

@oliverklee
Copy link
Collaborator Author

I changed this in the commit message to "conflated", which is possibly more accurate, though "intermingled" was fine.

Thanks - always glad to work with a native speaker! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants