Uff, took me a while to wrap my head around this but as someone who wrote a ton of library documentation with such snippet engines I felt I should give this a proper read -- hope the feedback can be fuel some productive discussion!
Thanks for the proposal, this is a great topic that couldâtogether with doccâfinally lead to great improvements in consistency and quality of documentation of swift libraries.
Having that said, I have some concerns about the current shape of what is being proposed.
The proposal states that:
- Complete sample projects
- Bits of code displayed inline within documentation
Both of these are critical tools, and snippets arenât intended to replace either of them. Instead, snippets offer an additional method for teaching with code, that fits somewhere between both ideas.
(Emphasis mine); My primary problem with the way this proposal positions itself is by not aiming to fix the "bits of code displayed inline" which are often written in-line in documentation in Swift documentation today, which leads to them being outdated (snippets don't even compile), or slightly wrong (snippets compile but do the wrong thing).
Shouldn't snippets aim to replace such "random snippets which don't even compile"? I'm sure you know that because you explain the issues with them in the Motivation section after all.
I'd like to improve on the
Each snippet is a single file, making it easy to think about as an author and as a reader, but it is also a fully valid program.
part of the proposal, based on my prior experience in writing a lot of reference documentation, first using rst and later on building our own documentation engine paradox that was used a lot by various lightbend/scala ecosystem libraries...
In one sentence my point here is that a snippet not just a file, but a file contains many snippets which are purposefully deliniated by the documentation author. The focus isn't on "hiding" but on showing concrete important pieces.
In many projects that are a bit more advanced than a collections library or something similar, you often end up interleaving "snippet" with "explanation", and then the "next step snippet" and so on.
In the current model, I would have to replicate all the "setup" to show the "next step" into many files -- as many, as I have steps in my explanation.
Rather, I would suggest adopting a "snippet is an identifier of a section of code" approach, like this:
//# imports // whatever the syntax might be
import Library
//# // "end current snippet", or tagged to "end a concrete snippet" with // snippet: imports
//# setup
let x = [...]
//#
//# step-2
x.example()
//#
This would be contained in a file like Snippets/Example.swift
. Then we'd include them like:
@Snippet("Intro", sections: "imports", "setup", "step-1", "step-2")
It is also worth discussing the @Snippet
as it is proposed today, as it uses "path
" which is a bit misleading; because it is not a real "path" (like file path). And since we're going to assume "Snippets/
" anyway might as well simplify it a bit, and just ask for the snippet name, and if we have to disambiguate, the module name:
@Snippet(/*module name*/"MyLibrary", /*file name*/"Intro", chunks: "...") // "chunks" or "snippets"
This "opt-in sections to be shown" approach also solves the question about imports really, since it is up to the snippet author to decide wether to include them and WHICH imports to include.
For example, I may be using my testkit library but I don't need the users to know about it since I'm using it in verification of the snippet:
//# imports
import Library
//#
import LibraryTestKit // NOT shown in snippets
//# sample
something()
//#
// @Snippet("...", sections: "imports", "sample")
The proposed use of // MARK: HIDE
seems weird to me. Again, i'd propose the opposite -- sections which are to be referenced to from prose documentation should be annotated, not the other way around. This way we can:
@Snippet("Intro", sections: "imports", "setup", "step-1", "step-2")
Where in our Intro.swift
we can have tests between all those steps:
//#imports
import CoolLibrary
import CoolLibraryExtras
//#
import XCTest
//# step-1
let thing = initializeThing()
//#
assert(thing.something == 2)
//# step-2
let result = await thing.performAction()
//#
XCAssertEqual(result, "something")
This way we're able to show the steps of an unfolding code snippet, and in the end show the entire thing if we wanted to.
Don't forget to import...
@Snippet("Hello", sections: "imports")
Then you can...
@Snippet("Hello", sections: "step-1")
And finally, we're ready to ...
@Snippet("Hello", sections: "step-2")
### Complete listing
@Snippet("Hello")
Nesting restriction
A single level of subdirectories is allowed to balance filesystem organization and further subdirectories for snippet-related resources, which are expected to be found informally using relative paths.
The "single level of subdirectory" seems like an arbitrary restriction, I'm not sure it's worth imposing it. You'd have to properly diagnose if a deeper structure is detected, and there's no real benefit from banning those. Why forbid Cluster/SpecificConfiguration/[...].swift
examples :-)
Testing
I have concerns about how testing is not really thoroughly discussed for snippets.
Effectively this proposal claims either "just compile them" or "just run them" and that would be enough to verify them... but the practical side isn't fleshed out.
Compiling snippets (swift build --build-snippets
as being proposed) we know isn't enough, as we need to run them as well -- but to do so, we have to list them and run them all "one by one", and the proposal does not offer any "test my snippets" solution.
Having to list all samples in some bash script to make sure they're tested on CI is not really acceptable, there must be an automatic way to make sure when I add new snippets that they're going to be tested already.
This is why usually such snippet systems I worked-with/built in the past, rely on tests to BE the source of snippets. So the discovery and "make sure they're all run" is solved by test runners. I don't get the argument about XCTest being platform specific as a reason for not being able to use these -- test discovery works on both Linux and Darwin and same do all assertions.
While XCTest may be lagging behind a bit on features sometimes (that's a different issue), I don't see why disallowing snippets from tests? E.g. we totally can / could:
//#imports
import DistributedActors // cluster lib
//#
import DistributedActorsTestKit
import XCTest
// test taken from http://github.com/apple/swift-distributed-actors
final class ClusterAssociationTests: ClusteredActorSystemsXCTestCase {
func test_snippet_join() throws {
let other = self.setUpNode()
//#joining-a-cluster
let cluster = ClusterSystem()
cluster.join(other.cluster.node)
//#
// Assert using lots of unit-test utils that would have to be made work in XCTest,
// as well as "in app for snippet" if snippets cannot be tests...
try assertAssociated(other, withExactly: cluster.uniqueNode)
}
}
While this is attempted to be addressed in "Tests acting as snippets" it doesn't really discuss it in depth enough IMHO.
The proposal, as is, offers a sub-par solution -- there's no "just test my snippets". And the only argument made against tests as snippet sources is that that simple top level script files are simpler, but that isn't really a very strong argument...
"MARK: HIDE"
The proposed snippet design where we can "hide" but not "select what to show" also makes it harder to show things to not do, which are often as useful to show as proper usage.
For example, with selecting snippets, I could:
func dontExecute() {
//#bad
thing.mode(.one)
try thing.two()
//#
}
func good() {
//#good
thing.mode(.two)
try thing.two()
//#
}
but rather we'd have to make a complete new file for the "bad" case, and then dance around with // MARK: HIDE
// MARK: HIDE
func dontDoThis() {
// MARK: SHOW
thing.mode(.one)
try thing.two() // don't do this
// MARK: HIDE
}
// MARK: SHOW
I also find it rather weird to use the MARK
syntax for this... as it results in outline elements in the "minimap" that are nonsensical (just a bunch of HIDE
)...
Swift Package Manager
This section is a bit light on content; A new target type "snippet
" is mentioned but not explained more, I think this deserves more details as introducing a new target type is rather substantial.
How are dependencies handled for snippets? Say I need to include some metrics implementation for my snippets to make sense; where do I declare that dependency? Is that overriding a SnippetGroup in the Package.swift, or somewhere else?
Finally, the proposal mentions "Multiple snippets per file." in future directions, but to me this is the heart of such engines, as explained above in depth, and I find it very lacking and limiting setting us off from the "wrong foot" onto this journey I think, personally -- documentation authors want to show exact pieces of a file to users, and not just dump an entire file at them (again, based on my experience documenting large/advanced libraries).
// edit: typo