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.

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

Incorrect md obtained from bold text with spaces around #62

Closed
4 tasks done
599316527 opened this issue Jun 6, 2024 · 7 comments
Closed
4 tasks done

Incorrect md obtained from bold text with spaces around #62

599316527 opened this issue Jun 6, 2024 · 7 comments
Labels
👍 phase/yes Post is accepted and can be worked on

Comments

@599316527
Copy link

599316527 commented Jun 6, 2024

Initial checklist

Affected packages and versions

mdast-util-to-markdown@2.1.0

Link to runnable example

https://codesandbox.io/p/sandbox/to-md-bold-text-with-around-spaces-ky39n9

Steps to reproduce

import { fromMarkdown } from "mdast-util-from-markdown";
import { toMarkdown } from "mdast-util-to-markdown";

const root = fromMarkdown("**A **");  // -> Strong "A "
const content = toMarkdown(root);          // -> "**A **"
const root2 = fromMarkdown(content);       // -> Text "**A **"

Expected behavior

Escape spaces on edges of bold text. (Bold text A -> **A **)

Or for better visual experience, shift edge spaces out of bold text. (Bold text A -> **A** )

Actual behavior

Spaces are not escaped on edges of bold text

Affected runtime and version

node@16.20.1

Affected package manager and version

npm@8.19.4

Affected OS and version

macOS Ventura 13.6.7

Build and bundle tools

webpack

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jun 6, 2024
@wooorm
Copy link
Member

wooorm commented Jun 6, 2024

Yes! This seems to be https://github.com/orgs/syntax-tree/discussions/60#discussioncomment-2111096. It’s quite complex.
It would need new features as it can’t be done with the current safe.js handling.
Perhaps in the text handler, but special to emphasis/strong (and GFM strikethrough/delete).

@wooorm wooorm added the 👍 phase/yes Post is accepted and can be worked on label Jun 6, 2024
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

@ChristianMurphy
Copy link
Member

Welcome @599316527! 👋
General advise don't put spaces on the inside edge of emphasis, is is very difficult for a human author to produce themselves or edit.

Non breaking spaces is not the same as a space.
It is possible to serialize this but it's complex https://github.com/orgs/syntax-tree/discussions/60#discussioncomment-2111096

IIRC this effort is part of #12

@599316527
Copy link
Author

Em, I just mean   here. Sorry for using nbsp to cause ambiguity.

JFYI. My usage scenario is to obtain the data structure from the rich text editor and then convert it to the md ast. In the rich text editor, it is a common operation for users to enter spaces around bold text. I currently manually shift the edge spaces out of bold text to bypass this problem.

@ChristianMurphy
Copy link
Member

I currently manually shift the edge spaces out of bold text to bypass this problem.

If the intent is for users to be able to understand and edit the markdown directly.
This would be a good idea to keep doing this. Escape characters, while valid, are confusing to some authors.

@wooorm
Copy link
Member

wooorm commented Jun 6, 2024

This would be a good idea to keep doing this. Escape characters, while valid, are confusing to some authors.

Yes, I agree with this, knowing that you are making a WYSIWYG editor experience.
If we solve this, your users will see  /  a lot and they will probably not like it, perhaps thinking it is a bug.
You’d still want to shift that space out yourself?

Copy link

github-actions bot commented Jun 7, 2024

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👍 phase/yes Post is accepted and can be worked on
Development

No branches or pull requests

3 participants