Skip to content

adds stripped state pre processor - #330

Open
PoTheMagicDragon wants to merge 3 commits into
lambda-client:1.21.11from
PoTheMagicDragon:stripped-logs-printer
Open

PoTheMagicDragon wants to merge 3 commits into
lambda-client:1.21.11from
PoTheMagicDragon:stripped-logs-printer

Conversation

@PoTheMagicDragon

Copy link
Copy Markdown
Contributor

Description

Adds a pre processor for handling stripped logs

@beanbag44
beanbag44 self-requested a review September 16, 2026 16:56

@beanbag44 beanbag44 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

small stuff

interface StateProcessor {
/** Checked before [acceptsState], so a disabled processor still falls back to breaking. */
context(_: AutomatedSafeContext)
fun isEnabled(): Boolean = true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the isEnabled check is only overridden by StrippedStateProcessor so the check should probably be moved into the acceptsState for StrippedStateProcessor and the function removed.

override fun isEnabled() = automatedSafeContext.buildConfig.stripLogs

override fun acceptsState(state: BlockState, targetState: BlockState): Boolean {
val unstrippedVariant = unstrippedToStripped[targetState.block] ?: return false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this function as a whole can probably be simplified to an = function without a full block


override fun acceptsState(state: BlockState, targetState: BlockState): Boolean {
val unstrippedVariant = unstrippedToStripped[targetState.block] ?: return false
return state.isReplaceable || state.block == unstrippedVariant

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

state.isReplaceable shouldnt be necessary here. Its best to have strict requirements for a preprocessor to accept. If the state is replaceable, the preprocessor doesnt need to pre process as its not a log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How do we make it place the unstripped log in the correct spot then?

if (processor.acceptsState(state, targetState)) {
with(processor) { preProcess(state, targetState, pos) }
stateProcessing = true
if (!processor.acceptsState(state, targetState)) return@forEach

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this shouldnt be altered

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants