Skip to content

docs, cmd: fix broken docs source links regression in vdoc#23889

Merged
spytheman merged 3 commits into
vlang:masterfrom
larpon:vdoc/fix-source-links
Mar 9, 2025
Merged

docs, cmd: fix broken docs source links regression in vdoc#23889
spytheman merged 3 commits into
vlang:masterfrom
larpon:vdoc/fix-source-links

Conversation

@larpon

@larpon larpon commented Mar 9, 2025

Copy link
Copy Markdown
Contributor

Locally running v doc -m -f html -o ./tmpdocs vlib/ && firefox tmpdocs/builtin.html now results in source links showed:
image

I'm unsure about how to add a regression test 🤔
Maybe we could do an assert somewhere if cfg.is_vlib == true 🤷

@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: V_0.6-22292

Comment thread cmd/tools/vdoc/vdoc.v Outdated
Comment on lines +316 to +317
} else if cfg.is_vlib {
assert false, 'vdoc: manifest does not exists for vlib'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe eprintln can function better here, followed by an exit as done above.

@larpon larpon Mar 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idea is to ensure that we do not let this happen again when the code base is worked upon - it is not (/should not be) a vdoc runtime error IMO. An invalid / unreadable manifest_file results in bad side-effects when vlib's own docs are generated, an assert is a way to prevent it longer term. I don't currently see how to put his in a test where it would probably fit better?

@spytheman spytheman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellent work.

@spytheman spytheman merged commit 81afd8f into vlang:master Mar 9, 2025
@larpon larpon deleted the vdoc/fix-source-links branch March 9, 2025 14:44
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