-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fix name clashes for auto-generated index files when using the same name for subdirectories #61
Fix name clashes for auto-generated index files when using the same name for subdirectories #61
Conversation
…ame for subdirectories
It has some conflicts as I merged the previous update. The replace function can be smarter I think, see this SO question: https://stackoverflow.com/questions/3210393/how-do-i-remove-all-non-alphanumeric-characters-from-a-string-except-dash |
# Conflicts: # src/DocNet/NavigationLevel.cs
I'd moved the logic in the property to create the name to a method as properties shouldn't be that big. This isn't a big deal. What is a bit of a big deal is that this changes root pages' filenames, even if they didn't have a clash before. This is a problem as documentation that already has been generated with previous versions of DocNet will get new filenames for auto-generated index files (we use these in our docs at places). This breaks links to the original pages (we have these in our docs) and that's something I want to avoid, especially as this is a fix for a problem that only occurs in some situations. So, sadly, the current situation can't change, the fix has to solve the problem without producing different files for the situation when there's no clash. It is a bug in the original code (the parent container's TargetUrl always results in "", so the filename is never prefixed with any kind of path), but one which we rely on ourselves, so abit unfortunate ;). I don't know of any elegant solution, to be honest :/ |
You can always update your current pages to be redirects to the new ones. |
There are many pages, and if I can avoid updating those (over 3 versions), I will. Not only that, but I can't fix external links to the docs, or other people's docs who use docnet and have docs generated by it. I'm sure there's a solution for this, but the current PR represents a change that I can't make. It's not all lost, perhaps the code can be re-used for a solution to the problem that doesn't break existing documentation sets. We just have to think abit about what solution will work :) |
Well, in theory your current docs should be considered broken. But let's assume they are not, then we could use flat files to have the same logic as before. Then we will only update auto-index files that are at least 1 level deep (but even then you'll have an issue where they might be broken). What we could do is auto-generate a forwarder on the old files that are possible clashes. Then if a user accidentally uses an old one, they will be forwarded to the new (but if broken, it's still broken). If they navigate through the new trees though, they will always use the new system. |
I disagree with your point that the current docs are broken, they're not, actually (they work fine). They will be broken when a documentation set is defined which results in duplicate files (due to the bug). If I keep things as they are now, documentation generated with it will work fine. The thing is that it won't generate proper files if this particular clashing situation is met, and for that situation we should find a solution. I find the forwarder solution not that great as off-line reading still won't work (and we ship the docs for offline reading). Let's look at a solution which fixes the clashing situation and which keeps the current situation as well. I don't mind a setting in the docs which can be enabled to get the filenames as you have defined in the PR. The only problem with that is that the filenames are created at a spot where the config file isn't reachable.... :/ |
Offline reading will work just fine with forwarding like this:
|
I'm not going to merge the breaking change, sorry. I see that the redirect might work, but it's a solution for a problem users currently don't have (as the bug isn't common, I haven't run into it in our large sets of docs and haven't heard from others as well). We'll have to think of something else. |
But I don't understand. The old situation will keep working? Then what's the breaking change? |
Please understand, I don't want any of this, I want a solution for when the clash happens and only then a file which doesn't clash should be generated. |
How about this:
This requires a fix for the bug where the path of the container is always "" at the moment and a setting which controls whether it will keep the value at "" or will use the path it should have. I'll think about what causes the real issue and see if I can solve it that way, with the setting as a way to control existing and fixed behavior. |
What if we generate this new structure only when clashes happen? We will need to expose this somehow and make it a 2-step generation process:
|
Ah, just noticed you also commented. A setting would work as well, but in your case instead of generating a longer file name, you will put it in the right folder. That would at least make the urls look much better than they do now, which would work as well in my case. |
good :) I'll first investigate what's the cause of the bug, then see what can be done about it and whether it's more work to fix that or to do a full tree-first approach as you suggested. Will try that tomorrow :) |
The bug is more subtle. It's not that everything is generated into the root, it's that there's a lack of a delimiter. I have this json in my docnet.json file: "Pages":
{
"__index": "index.md",
"Welcome!": "docconventions.md",
"What's new in v5.2": "whatsnew.md",
"Migrating your code": "migratingcode.md",
"LLBLGen Pro Runtime Framework features": "supportedfeatures.md",
"Getting Started" :
{
"__index": "getting_started.md",
"Quick start guides" :
{
"From Database to source code":
{
"__index": "qsg\\dfscenario1\\index.md",
"Create a project" : "qsg\\dfscenario1\\CreateProject.md",
"Create entities based on a database": "qsg\\dfscenario1\\df_reverseengineer.md",
"Generate source code":"qsg\\dfscenario1\\generatesourcecode.md"
},
"From nothing to database and source code":
{
"__index": "qsg\\mfscenario1\\index.md",
"Create a project" : "qsg\\mfscenario1\\CreateProject.md",
"Create an entity model":
{
"Choice 1: Quick Model": "qsg\\mfscenario1\\AddEntityQuickModel.md",
"Choice 2: Manual adding through entity editor": "qsg\\mfscenario1\\AddEntityEntityEditor.md",
},
The 'Create an entity model' section doesn't have an index page defined, so one will be generated automatically. It will be generated as: For some pages this will be they'll end up in a folder higher up than they're meant to be, which is the root (as the root always has an index page otherwise the tool stops). The problem is here: https://github.com/FransBouma/DocNet/blob/master/src/DocNet/NavigationLevel.cs#L307, in the string.Format() call. It simply concats the path in front of the file, so the delimiter is lost. Have to think about how to fix this properly w/o breaking anything. |
Fixing this shows up some nasty edge cases the author of docs likely won't think of: "Linq to LLBLGen Pro":
{
"Getting started": "Using the generated code\\Linq\\gencode_linq_gettingstarted.md",
"General usage": "Using the generated code\\Linq\\gencode_linq_generalusage.md",
"Prefetch paths": "Using the generated code\\Linq\\gencode_linq_prefetchpaths.md",
"Function mappings": "Using the generated code\\Linq\\gencode_linq_functionmappings.md",
"Remarks and limitations": "Using the generated code\\Linq\\gencode_linq_remarkslimitations.md",
}, If I do the 'right thing' which is: Using so in your case, this solves it: {
"Name" : "DocNet repro documentation",
"Source" : ".",
"Destination" : "../",
//"IncludeSource": "Includes",
"Theme" : "Default",
"ConvertLocalLinks" : true,
//"SourceFoldersToCopy" : ["images", "includes"],
"Footer" : "",
"Pages" :
{
"Somestuff 1" :
{
"__index": "somestuff1/index.md",
"Introduction" :
{
"Getting started" : "somestuff1/introduction/getting-started/page1.md",
},
},
"Somestuff 2" :
{
"__index": "somestuff2/index.md",
"Introduction" :
{
"Getting started" : "somestuff2/introduction/getting-started/page2.md",
},
}
}
} This gives the parent "Somestuff 1" a folder, through its __index parameter. Also with the current code, as it will generate the foldername into the filename (so there will be 2 files: somestuff1Introduction.htm and somestuff2Introduction.htm) So adding the delimiter fix makes the file move to a folder of the container, which breaks current docs, and won't fix all scenarios and thus isn't a proper solution either (besides, it's hard to make this configurable). To fix #57 the parent container has to get an index so it has a folder, like described above. The issue isn't solve with that tho, it's still there, but sadly a side effect of how things are organized in the tool at this point. (best way would have been perhaps to assign folders to sections and all files in a section inherit the folder of the section, so there never would be a folder mismatch like I described earlier too). I'll close this PR as it's not going to be merged. I'll add a new issue referencing to this PR and the posts above to document that it's an open, known issue and that a workaround exists. |
Thanks for the research and detailed explanation. Since these are auto-generated docs (using the reference/**, there is no real workaround for me I guess). I understand that you don't want to merge this pr. |
Please see #64 as I think there is a solution, but it's more drastic than a filechange... |
Fixes #57