Skip to content
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.

Sign up for GitHub

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

Jump to bottom

vmui/logs: add markdown support #6292

Merged
merged 6 commits into from
Jun 10, 2024
Merged

vmui/logs: add markdown support #6292

merged 6 commits into from
Jun 10, 2024

Conversation

Loori-R
Copy link
Contributor

@Loori-R Loori-R commented May 16, 2024 •
edited

Describe Your Changes

Add support for markdown format and emoji for the _msg field in the "Group" view.
Add markdown rendering toggle. Disabled by default. Value is stored in localStorage.

before
after

Checklist

The following checks are mandatory:

@Loori-R Loori-R added the vmui label May 16, 2024
dmitryk-dk
dmitryk-dk approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@dmitryk-dk dmitryk-dk left a comment

Choose a reason for hiding this comment

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

LGTM!

@zekker6
Copy link
Contributor

zekker6 commented May 16, 2024 •
edited

Do I understand it correctly that current implementation enables markdown rendering by default and there is no way to disable it?
I'm afraid that there will definitely be use-cases when this will render markdown at places where it was just using *, _ or other special characters in markdown, so it would be great to have a toggle to disable this.

@Loori-R
Copy link
Contributor Author

Loori-R commented May 17, 2024

@zekker6, good idea, I'll add a switch.
Should we disable it by default?

@zekker6
Copy link
Contributor

zekker6 commented May 17, 2024

@zekker6, good idea, I'll add a switch. Should we disable it by default?

From my experience, it's better to disable this by default as most logs do not contain markdown at all.

It would be great if we could enable this by default for the play environment thought (or have some kind of automatic detection logic in place).

@hagen1778
Copy link
Collaborator

@Loori-R could you please resolve conflicts in this PR?

@Loori-R
Merge branch 'master' into vmui/logs/markdown-support
f981dbe 
# Conflicts:
#	app/vmui/packages/vmui/src/pages/ExploreLogs/ExploreLogsBody/ExploreLogsBody.tsx
#	app/vmui/packages/vmui/src/pages/ExploreLogs/ExploreLogsBody/GroupLogs.tsx
#	app/vmui/packages/vmui/src/pages/ExploreLogs/ExploreLogsBody/style.scss
#	docs/VictoriaLogs/CHANGELOG.md
@Loori-R
vmui/logs: resolve conflicts with master
6f5812d
@Loori-R
Copy link
Contributor Author

Loori-R commented Jun 10, 2024

@hagen1778 Resolved conflicts

hagen1778
hagen1778 approved these changes Jun 10, 2024
View reviewed changes
Copy link
Collaborator

@hagen1778 hagen1778 left a comment

Choose a reason for hiding this comment

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

LGTM

@hagen1778 hagen1778 merged commit 84088e5 into master Jun 10, 2024
8 checks passed
@hagen1778 hagen1778 deleted the vmui/logs/markdown-support branch June 10, 2024 14:38
valyala pushed a commit that referenced this pull request Jun 10, 2024
@Loori-R @valyala
vmui/logs: add markdown support (#6292)
2300e30 
Add support for markdown format and emoji for the `_msg` field in the
"Group" view.
Add markdown rendering toggle. Disabled by default. Value is stored in
`localStorage`.
@valyala
Copy link
Collaborator

valyala commented Jun 11, 2024

FYI, this pull request has been included in v0.19.0-victorialogs.

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

@hagen1778 hagen1778 hagen1778 approved these changes

@dmitryk-dk dmitryk-dk dmitryk-dk approved these changes

@valyala valyala Awaiting requested review from valyala

Assignees
No one assigned
Labels
victorialogs vmui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Loori-R @zekker6 @hagen1778 @valyala @dmitryk-dk @Amper