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

RichText swallowing whitespace after Markdown links #4613

Closed
2 tasks done
sdassow opened this issue Feb 5, 2024 · 6 comments
Closed
2 tasks done

RichText swallowing whitespace after Markdown links #4613

sdassow opened this issue Feb 5, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@sdassow
Copy link
Contributor

sdassow commented Feb 5, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

Spaces and linebreaks following a markdown link in a RichText widget get swallowed.

How to reproduce

  1. Run example code
  2. See missing whitespace after link

Screenshots

Missing space after link:
fyne-richtext-url-squeeze

Example code

package main

import (
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/widget"
)

var md = `# headline for a wide window

[https://docs.fyne.io/started/hello](https://docs.fyne.io/started/hello)

foo

bar
`

func main() {
	a := app.New()
	w := a.NewWindow("Hello World")

	rt := widget.NewRichTextFromMarkdown(md)

	w.SetContent(rt)
	w.ShowAndRun()
}

Fyne version

2.4.3

Go compiler version

1.21.1

Operating system and version

OpenBSD 7.4

Additional Information

No response

@sdassow sdassow added the unverified A bug that has been reported but not verified label Feb 5, 2024
@dweymouth
Copy link
Contributor

dweymouth commented Feb 6, 2024

Can you test on develop? There was a bug (#3528) that is fixed on develop for the next release, at least for widget.Hyperlink but I actually don't remember if it fixed it in RichText as well (I think so). The swallowing spaces issue may be a new one though.

@sdassow
Copy link
Contributor Author

sdassow commented Feb 9, 2024

Turns out those are two issues after all, my bad. The one about the link is indeed fixed in develop.
Going to change the description so that it's only about swallowing the spaces.

@sdassow sdassow changed the title RichText weirdness with Markdown links RichText swallowing whitespace after Markdown links Feb 9, 2024
@codesoap
Copy link
Contributor

I've just encountered a similar issue and after some investigation I think there is a more general problem. There is special logic in the addTextToSegment function, which adds spaces between text segments. This code does not consider other inline elements, like emphasis, strong text and links.

Demo

package main

import "fyne.io/fyne/v2/app"
import "fyne.io/fyne/v2/widget"

func main() {
        a := app.New()
        w := a.NewWindow("test")
        r := widget.NewRichTextFromMarkdown(`foo
[bar](https://bar.com)
baz.

foo
*emph*
baz

foo
**strong**
baz

[split
link](https://bar.baz)

This works,
though.`)
        w.SetContent(r)
        w.ShowAndRun()
}

Currently produces this (the "split link" example may also show another bug):
newline

Tests

I have prepared a fix for the missing spaces around links, but now that I see the more general problem, I'm not sure this fix is the right way. Maybe a deeper change in the markdown parsing code is required (especially for the split link problem). However, I can already contribute some tests for widget/markdown_test.go:

func TestRichTextMarkdown_NewlinesAroundEmphasis(t *testing.T) {
	r := NewRichTextFromMarkdown("foo\n*bar*\nbaz")
	assert.Equal(t, "foo bar baz", r.String())
}

func TestRichTextMarkdown_NewlinesAroundStrong(t *testing.T) {
	r := NewRichTextFromMarkdown("foo\n**bar**\nbaz")
	assert.Equal(t, "foo bar baz", r.String())
}

func TestRichTextMarkdown_NewlinesAroundHyperlink(t *testing.T) {
	r := NewRichTextFromMarkdown("foo\n[bar](https://fyne.io/)\nbaz")
	assert.Equal(t, "foo bar baz", r.String())
}

func TestRichTextMarkdown_NewlineInHyperlink(t *testing.T) {
	r := NewRichTextFromMarkdown("[foo\nbar](https://fyne.io/)")
	assert.Equal(t, "foo bar", r.String())
}

@andydotxyz
Copy link
Member

Looks like you're on to something here thanks.
I wonder if the "split link" could be either an issue with the parser we use, or maybe not allowed in the spec (not sure).

@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Feb 25, 2024
@andydotxyz andydotxyz added this to the D fixes (v2.4.x) milestone Feb 25, 2024
@codesoap
Copy link
Contributor

@andydotxyz FYI: I've started looking into fixing this (including the "split link"). Just a heads up, so that we don't do the same work in parallel by accident.

codesoap added a commit to codesoap/fyne that referenced this issue Feb 26, 2024
The new version simplifies the code by using recursion instead of
walking the ast while keeping track of state.

It also improves the code by using a type switch.

Fixes fyne-io#4613
codesoap added a commit to codesoap/fyne that referenced this issue Feb 26, 2024
The new version simplifies the code by using recursion instead of
walking the ast while keeping track of state.

It also improves the code by using a type switch.

Fixes fyne-io#4613
codesoap added a commit to codesoap/fyne that referenced this issue Feb 26, 2024
The new version simplifies the code by using recursion instead of
walking the ast while keeping track of state.

It also improves the code by using a type switch.

Fixes fyne-io#4613
andydotxyz added a commit that referenced this issue May 7, 2024
Rework markdown parser to fix #4613
@andydotxyz
Copy link
Member

Fixed on develop ready for v2.5.0 thanks to @codesoap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants