Skip to content

Conversation

@xkr47
Copy link
Contributor

@xkr47 xkr47 commented Feb 17, 2020

Includes new configuration option git-repository-edit-baseurl for
supporting non-GitHub repository layouts.

Includes new configuration option `git-repository-edit-baseurl` for
supporting non-GitHub repository layouts.
@xkr47 xkr47 force-pushed the feature/suggest-an-edit-link branch from 2f99a45 to cf4cb53 Compare February 17, 2020 21:02
@ehuss
Copy link
Contributor

ehuss commented Feb 17, 2020

I would prefer if this wasn't enabled by default.

Also, the default of /blob/master results in broken links if the git-repository-url isn't the root of a github project. I don't think we should assume people are using github, or have the book in the root of the repo, or are pointing links to the project root.

I also don't quite see the point of a "suggest an edit" link that just takes you to the source browser. It seems to be very similar to the existing git repo link.

I wonder if using a github /edit/ link would make more sense? Or maybe just make some general mechanism for easily adding arbitrary icons of any kind?

@xkr47
Copy link
Contributor Author

xkr47 commented Feb 17, 2020

Well the repository link already defaulted to the github icon, so I thought assuming github was ok. But you have valid point.

I think it is good to have a direct link to the chapter being viewed, so the random reader won't have to figure out the repo directory structure and file naming conventions to contribute. I'm perfectly fine with changing the link to point to the edit page directly. Otoh if we won't assume github, which means the user always has to enter the edit baseurl separately, we won't be assuming anything about the url anyway so it's just up to documentation to give good examples?

@xkr47
Copy link
Contributor Author

xkr47 commented Feb 17, 2020

Bitbucket format for edit urls is https://bitbucket.org/<user>/<repo>/src/<branch>/<path>?mode=edit so maybe we need to use a template string instead of base url?

@ehuss
Copy link
Contributor

ehuss commented Feb 17, 2020

Making git-repository-url a template with something like {path} substituting the relative path to the source file sounds good. That would allow the book author to decide exactly what they want to do (always link to the root, link to the source page, link to an edit page, etc.).

@xkr47
Copy link
Contributor Author

xkr47 commented Feb 17, 2020

Having a separate setting for editing chapters/pages that solely controls the visibility of the "edit" icon would let the owners decide whether they want repo link, a direct edit link, or both.. WDYT?

Will try to get this done soon, this is my first day coding rust after all :)

@xkr47 xkr47 force-pushed the feature/suggest-an-edit-link branch from 80e1f4c to 033d04f Compare February 17, 2020 22:42
@ehuss
Copy link
Contributor

ehuss commented Mar 9, 2020

The index pages seem to be broken. For example, in the book-example adding:

git-repository-edit-url-template = "https://github.com/rust-lang/mdBook/edit/master/book-example/{path}"

the front page tries to load index.md instead of README.md. Same with if you go to cli/index.html it tries to edit cli/index.md instead of cli/README.md.

@xkr47
Copy link
Contributor Author

xkr47 commented Mar 10, 2020

Good catch, I will investigate when I can!

@ehuss ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 21, 2020
@netlander
Copy link

Any idea when this is going to get merged?

@rubenmoor
Copy link

Bitbucket format for edit urls is https://bitbucket.org/<user>/<repo>/src/<branch>/<path>?mode=edit so maybe we need to use a template string instead of base url?

The exact url you can put in the index.hbs to keep the config file clean. All that I am missing really is a {{ md-file-path }} in the index.hbs that puts in the markdown-file, the <path>. Shouldn't that also be easier all in all?

@flavio
Copy link
Contributor

flavio commented Apr 12, 2021

Any idea when this is going to get merged?

I'm interested in this PR too. I see the PR got 1 approval, is that just a matter of doing a rebase and solving the conflicts? I would be happy to help with that if that's the case

@ehuss
Copy link
Contributor

ehuss commented Apr 12, 2021

@flavio it looks like there were still some issues with index handling. If you want to take it and try to fix that, I think that would be fine since it has been over a year since we last heard from the author.

@ehuss
Copy link
Contributor

ehuss commented Apr 26, 2021

This has been merged as #1506.

@ehuss ehuss closed this Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants