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

Added support for an alternative current dir mode in pkldoc #824

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

netvl
Copy link
Contributor

@netvl netvl commented Nov 18, 2024

Some systems, for example, GitHub Pages, have trouble with handling symlinks, which breaks the current directory links created by Pkldoc. In this PR, we add an alternative mode which creates a full copy of the latest published version contents in the current directory instead.

The directory where generated documentation is placed.
====

.--current-directory-mode
Copy link
Contributor

@odenix odenix Nov 19, 2024

Choose a reason for hiding this comment

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

Unless you foresee a third mode, a flag would be easier to use and understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that it might be possible to eventually have a mode to disable current generation at all.

Also, I'm not very sure about a binary flag to be easier to understand to be honest. It is not disabling or enabling something here, it is changing the logic of operation between two states. Enums are supposed to be the tool for this (one of discussions).

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @translatenix here; I think this should be called --no-symlinks, and default to false. It's not necessarily about how the "current" dir is treated; the intention is to disable symlinks.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point that I had failed to notice.

@netvl netvl force-pushed the current-publishing-variants branch from 2eca683 to 3af4b39 Compare November 19, 2024 20:46
Copy link
Contributor

@bioball bioball left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Per the comments, can we change this to a boolean flag? I think a name like --no-symlinks would be sensible here.

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