Skip to content

Add kind=(step, ) for root messages with *#973

Open
abitrolly wants to merge 5 commits intopypa:mainfrom
abitrolly:ctx-slog
Open

Add kind=(step, ) for root messages with *#973
abitrolly wants to merge 5 commits intopypa:mainfrom
abitrolly:ctx-slog

Conversation

@abitrolly
Copy link
Contributor

@abitrolly abitrolly commented Jan 5, 2026

This separates log semantic and representation.

Right now the first line printed with _ctx.log gets bold is prefixed '* ', and subsequent lines in the same message are indented. This makes things more complicated than necessary. It is more convenient to just print step messages bold and prefixed, and plain _ctx.log(message) as plain text.

@abitrolly
Copy link
Contributor Author

I propose to merge main logger functionality into _ctx. The _ctx default logger is only used in tests, and its behavior is different. Tests can mock logging if needed as everything else.

I also propose to replace origin tuple with plain string for readability.

@gaborbernat
Copy link
Contributor

Conflicting can you rebase?

@abitrolly
Copy link
Contributor Author

@gaborbernat rebased, but there are errors now.

@abitrolly
Copy link
Contributor Author

@gaborbernat fixed. I would actually prefer origin="step" syntax as it makes more readable code. Or I can send another PR.

@layday
Copy link
Member

layday commented Mar 4, 2026

Indenting log lines without an origin doesn't appear semantically correct to me, nor is "step" an origin per se. The logger context attempts to accommodate both regular logging and bypassing stdlib logging to print informational messages to stderr. Splitting dependencies into separate messages wouldn't make sense for the former use case.

_cprint('{bold}{}{reset}', fill(first, initial_indent='* '), file=sys.stderr)
for line in rest:
print(fill(line, initial_indent=' '), file=sys.stderr)
print(message, file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

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

You removed fill so these lines won't be wrapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any software that wraps log lines itself? I always thought it is a job of a terminal/viewer.

For log analysis it is better no have output that doesn't depend on terminal width, if, for example, need one to save it for comparison later.

So why maintain this extra code if terminal already does it for us?

Copy link
Member

Choose a reason for hiding this comment

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

If the lines are indented, then yes, they should be wrapped by the tool. That's another reason not to split these messages up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@layday I put the fill back and renamed log message origin to log message kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fill also removes linefeeds, and it garbles multiline output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests are fixed. Should be good now.

@abitrolly abitrolly changed the title Add origin=(step, ) for root messages with * Add kind=(step, ) for root messages with * Mar 6, 2026
For example, `step` is not really an origin, but a type of log
message that needs to be highlighted.
@abitrolly
Copy link
Contributor Author

@layday I replaced message origin with more generic message kind, which can also be used for debug messages in increased verbosity.

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