Support automatic inclusion of files using (e.g. reference/) #58

GeertvanHorrik · 2017-06-12T19:23:23Z

Fixes #48

Usage is fairly simple (in DocNet):

"Pages" : 
    {
		"Introduction" :
		{
			"Introduction" : "introduction/introduction.md"
                },
		"FAQ" : 
		{
			"General" : "faq/general.md",
			"MVVM" : "faq/mvvm.md",
			"Problem solving" : "faq/problem-solving.md",
			"Performance considerations" : "faq/performance-considerations.md",
		},
                "API reference" : "reference/**"
    }

The API reference part uses the ** at the end. It means that all (sub)folders inside /reference will be checked for MD files. A few automatic rules:

Auto index files will always be generated
The title of the md files will be retrieved from the actual MD files (so the first non-empty line will be used as title)

This becomes extremely useful when parts of the docs are generated via a tool such as SharpDox.

FransBouma · 2017-06-12T19:47:22Z

Could you write a couple of documentation lines to the docs in the gh-pages branch? Thanks :)

GeertvanHorrik · 2017-06-13T08:28:17Z

See #60

FransBouma · 2017-06-14T07:52:19Z

Due to the tabs vs. spaces whitespacing (code used tabs, you used spaces) git thinks the whole navigationlevel file has changed, which isn't the case. I can't see properly what has changed in the file to review it. Will try to revert spaces to tabs and then do a diff.

GeertvanHorrik · 2017-06-14T08:02:51Z

Sorry, didn't notice that. Probably the habit of hitting CTRL + K + D when editing lots of stuff in a file. Normally I try to accept the coding standard of the original project.

FransBouma

Can you make the changes to this file? I'll then merge it. If tabs/spaces are a problem, leave them as spaces, I'll correct it here. You don't have to remove the spaces around the () in while/if statements.

Thanks!

FransBouma · 2017-06-14T07:59:09Z

src/DocNet/NavigationLevel.cs

-		public bool IsRoot { get; set; }
-		#endregion
-	}
+    public class NavigationLevel : NavigationElement<List<INavigationElement>>


the whitespace issues make it hard to review the code. I should have added an editor config file but that's not honored in vs2015 anyway, but nevertheless, things are a bit problematic for reviewing. I can convert the spaces back to tabs tho, it's not the work, but to see which lines are changed.

Sorry, didn't notice that. Probably the habit of hitting CTRL + K + D when editing lots of stuff in a file. Normally I try to accept the coding standard of the original project.

It's ok, notepad++ happily converts spaces to tabs when you ctrl-a -> TAB, SHIFT TAB :) After thatn winmerge was happy to show the differences

Converted to tabs.

FransBouma · 2017-06-14T08:00:35Z

src/DocNet/NavigationLevel.cs

-	}
+    public class NavigationLevel : NavigationElement<List<INavigationElement>>
+    {
+        private readonly string _rootDirectory;


All my code has private member fields and properties in their own regions so these things are hidden when you're normally coding.

FransBouma · 2017-06-14T08:01:52Z

src/DocNet/NavigationLevel.cs

+
+        public void Load(JObject dataFromFile)
+        {
+            foreach (KeyValuePair<string, JToken> child in dataFromFile)


Nitpick: you introduced spaces in front of '(' for loops/ifs etc. Likely due to reformatting by a rule in Resharper. Code isn't bad due to that, but next time try to avoid these.

(don't need to change these for merge btw)

FransBouma · 2017-06-14T08:03:34Z

src/DocNet/NavigationLevel.cs

+                    var childValue = child.Value.ToObject<string>();
+
+                    var endsWithWildcards = childValue.EndsWith("**");
+                    if (endsWithWildcards)


endsWithWildcards is used just once, you can inline the variable in the if, which is preferable.

FransBouma · 2017-06-14T08:13:37Z

src/DocNet/NavigationLevel.cs

+                    continue;
+                }
+
+                var relativeFilePath = GetRelativePath(_rootDirectory, mdFile);


There's a MakeRelativePath in Utils.cs in the Docnet project, please use that one.

FransBouma · 2017-06-14T08:14:52Z

src/DocNet/NavigationLevel.cs

+            return root;
+        }
+
+        private string GetRelativePath(string origin, string fullPath)


Please remove and use Utils.MakeRelativePath instead

FransBouma · 2017-06-14T08:17:59Z

src/DocNet/NavigationLevel.cs

+        }
+
+        private string FindTitleInMdFile(string path)
+        {


At first I thought "We have a markdown parser, why use this!?" but it's indeed a catch-22: you can't use the title from the parsed content like I do with the H2's, as the content might not be parsed yet and the name is used in toc parts on pages where the markdown of this page isn't known (yet). It feels like a hack still, but I don't know of a different way to solve it, so let's keep it.

I am not extremely happy with this one either, but looked like the least-worst solution.

FransBouma · 2017-06-14T10:07:04Z

Merged!

Support automatic inclusion of files using ** (e.g. reference/**)

613c584

FransBouma requested changes Jun 14, 2017

View reviewed changes

Changes after code review

f516b93

FransBouma approved these changes Jun 14, 2017

View reviewed changes

FransBouma merged commit f516b93 into FransBouma:master Jun 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support automatic inclusion of files using (e.g. reference/) #58

Support automatic inclusion of files using (e.g. reference/) #58

GeertvanHorrik commented Jun 12, 2017

FransBouma commented Jun 12, 2017

GeertvanHorrik commented Jun 13, 2017

FransBouma commented Jun 14, 2017

GeertvanHorrik commented Jun 14, 2017

FransBouma left a comment

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

GeertvanHorrik Jun 14, 2017

FransBouma Jun 14, 2017

GeertvanHorrik Jun 14, 2017

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

FransBouma Jun 14, 2017

GeertvanHorrik Jun 14, 2017

FransBouma commented Jun 14, 2017

Support automatic inclusion of files using ** (e.g. reference/**) #58

Support automatic inclusion of files using ** (e.g. reference/**) #58

Conversation

GeertvanHorrik commented Jun 12, 2017

FransBouma commented Jun 12, 2017

GeertvanHorrik commented Jun 13, 2017

FransBouma commented Jun 14, 2017

GeertvanHorrik commented Jun 14, 2017

FransBouma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FransBouma commented Jun 14, 2017

Support automatic inclusion of files using (e.g. reference/) #58

Support automatic inclusion of files using (e.g. reference/) #58