Skip to content

Conversation

kstrafe
Copy link

@kstrafe kstrafe commented May 12, 2025

When we define our own --markers='x-begin x-middle x-end', and our code contains as a first character x, then that character will be considered part of the common prefix and be removed from the code. Here's an example.

// cog-begin
// cog.outl('hello world')
// cog-middle
// cog-end

The common prefix for this code is // cog, which makes the code generator fail because python will receive the code .outl('hello world'), which makes no sense.

This issue is especially noticable if you're only invoking a module, for instance:

// cog-begin
// codegen.generate_some_code()
// cog-middle
// cog-end

Here the python interpeter will receive degen.generate_some_code() and fail.

This patch removes the begin and end specs from the marker line to avoid this issue.

When we define our own --markers='x-begin x-middle x-end', and our code contains as a first
character `x`, then that character will be considered part of the common prefix
and be removed from the code. Here's an example.

    // cog-begin
    // cog.outl('hello world')
    // cog-middle
    // cog-end

The common prefix for this code is `// cog`, which makes the code generator fail
because python will receive the code `.outl('hello world')`, which makes no
sense.

This issue is especially cumbersome to work with if you're invoking a module,
for instance:

    // cog-begin
    // codegen.generate_some_code()
    // cog-middle
    // cog-end

Here the python interpeter will receive `degen.generate_some_code()`.

This patch removes the begin and end specs from the marker line to avoid this
issue.
@nedbat
Copy link
Owner

nedbat commented May 13, 2025

Help me understand: why not choose markers that are more unique?

@kstrafe
Copy link
Author

kstrafe commented May 13, 2025

Help me understand: why not choose markers that are more unique?

For aesthetics.

As a bonus, this will make cog support languages where no marker can satisfy the guarantee of uniqueness to prevent common prefix issues.

I also think it makes intuitive sense to start looking for the common prefix right before the begin/end spec substrings. These will never actually be part of a common prefix because you cannot have two begin specs in a row as cog returns an error.

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.

2 participants