[docfx] adding --watch option to build command

[rmannibucau]

Follow-up of my tests (#9986) and #9992

Not 100% sure of the following so happy to get a feedback on all these points:

1. didn't see much tests of the build/serve mode so does it need a test (regarding project compliance)?
2. I'm not very clear why default command is not build command but a kind of fork not doing exactly the same thing so maybe I didn't patch the right location
3. the glob to pattern logic is very simple to start (can always be enhanced later), would be happy to get an ack it is sufficient to start or not from your experience (I'm very new to docfx and don't know all the subtleties yet)
4. I opened docfx.json model, hope it is ok (I assumed thanks to Ensure BuildOptions can configure the output directory instead of forcing inline rendering #9986 it was maybe an option but not 100% sure)

[codecov]

Codecov Report

Attention: Patch coverage is 13.74046% with 113 lines in your changes missing coverage. Please review.

Project coverage is 78.48%. Comparing base (fe673ec) to head (74b6c34).
Report is 215 commits behind head on main.

Files	Patch %	Lines
src/docfx/Models/WatchCommand.cs	0.00%	107 Missing ⚠️
src/docfx/Models/DefaultBuildCommandOptions.cs	85.00%	3 Missing ⚠️
src/docfx/Models/WatchCommandOptions.cs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10010      +/-   ##
==========================================
+ Coverage   74.31%   78.48%   +4.16%     
==========================================
  Files         536      543       +7     
  Lines       23189    23590     +401     
  Branches     4056     4078      +22     
==========================================
+ Hits        17234    18514    +1280     
+ Misses       4853     3934     -919     
- Partials     1102     1142      +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[filzrev]

This change seems no behavior differences.
Access to this class from docfx is already allowed by InternalsVisibleTo attribute.

[rmannibucau]

Right but it helps #9986 too and makes this PR more "readable" since it does not need to check the InternalsVisibleTo so think it is worth it to include it - but if blocking I can remove it, trying to explain the rational.

[filzrev]

Default class visibility is internal when declared under namespace.
It thought it need to specify public for consistency with PR comments. (or wait #8872 is resolved)

https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/access-modifiers#class-and-struct-accessibility

Classes and structs declared directly within a namespace (aren't nested within other classes or structs) can be either public or internal. internal is the default if no access modifier is specified.

[filzrev]

In my personal opinion.
This feature should be implemented as separated docfx watch command (#2297).
And existing docfx and docfx build commands are leave as is.

Because docfx watch feature requires different set of options/configs.
And implementation can be simpler for future command extensibilities.

For example.

• Watch _site contents and reload browser window (LiveReload/BrowserRefresh) similar to dotnet watch command.
• Support Ctrl+R hotkey to force rebuild. similar to dotnet watch command.
• Support pseudo incremental build (Build only changed document)

[rmannibucau]

Because docfx watch feature requires different set of options/configs.
And implementation can be simpler for future command extensibilities.

Hmm, it has serve capabilities so adding watch which is a serve companion or concurrent is not shocking to me.
To be concrete if we do a watch command and follow the reasoning it means watch will not serve right? But then we need yet another watch+serve command so not sure it is great.
I'm coming from a projects where commands are build (plain build no serve or watch command) and serve or watch which both handle both features (optionally with live reload support) so I'm surely biased but my experience with docfx is I have no idea how serve can work since to work on the doc I needed to setup inotify and a plain http-server was replacing serve command quite quickly so maybe I'm missing some usage there - happy to learn if so.

For example.

Watch _site contents and reload browser window (LiveReload/BrowserRefresh) similar to dotnet watch command.

Kind of requires serve to be there as well no?

Support Ctrl+R hotkey to force rebuild. similar to dotnet watch command.
Support pseudo incremental build (Build only changed document)

+1 (it is the last else block where watcher is started without serve feature) and I'm happy to do a watch only feature extracted from from this PR but we still need an all in one command, where would it fit if not build one which is already an all in one?
Also what about the default command convergence with build or should it be used as all in one command?

[filzrev]

Thank you for confirming my PR comments

I'm expecting following behaviors by docfx watch command. (If it's added by this PR)

When docfx watch command executed.
By default, following processes are executed on separated HostedService thread.

1. Serve _site directory content with ASP.NET. (And inject livereload-js scripts to HTML file)
2. Watch changes of _site directory files. When changes are detected. Send LiveReload command to browser.
3. Watch changes of docs files. And run build when files are modified.
4. Watch keyboard input. And When Ctrl+R key pressed. Request docfx build.

And by adding watch section to docfx.json.
It can change behaviors.

• "Serve": false option suppress 1. and 2. behaviors.
• "Watch": false option suppress 3. behavior.

---

If basic docfx watch command infrastructure is added by this PR.
I'm also willing creating PR that relating to this features. (Ctrl+R hotkey support and LiveReload supports)

[rmannibucau]

@filzrev I'm in the process of moving build to watch command (restoring old build command), hope I got it right

[rmannibucau]

Anything I can do to help getting this on board?

[filzrev]

Anything I can do to help getting this on board?

To supporting docfx watch command.
I thought It need to add following statement to Program.cs.

config.AddCommand("watch");

And for WatchCommandOptions.
Following options expected to be enabled by default (that similar to dotnet watch command).

• Watch
• Serve
• OpenBrowser

Currently it inherited BuildCommandOptions though
Personally I though dotnet watch command should have different set of command line parameters.
(e.g. --no-watch)

@yufeih
I'm also want to add feature that relating dotnet watch command.
Is it able to create dedicated feature branch?

[rmannibucau]

Yes addcommand is needed but was awaiting to ensure solution was ok.

About build command i guess it would be neat to extract settings in a shared base setting class to ensure it is consistent accross command and not duplicated, wdyt?

edit: created a shared build command options class to host what is shared, moved watch/serve/openbrowser to command specific option classes and set watch/serve to true by default on watch command (not openbrowser since this one is more bothering than helping if you restart the command from time to time). Not sure the intent of --no-watch for a watch command so didn't tackled it - assuming it is exclusions, I think we would need another pass after this PR can be merged to work on the glob to watch pattern conversion but sounds a lot to do it all at once.

[yufeih]

if the build invoked by onChange method takes a minute to finish and files changes each second, would we trigger multiple build or wait for the last build to complete before triggering the next?

[yufeih]

@filzrev @rmannibucau

Implemented as a standalone watch command looks good. I added a feature/watch branch for you to work towards completion, also left some comments.

[yufeih]

We can start with a simple deterministic watch implementation that watches all files changes in the docfx.json directory, except for the output directory (_site)