Skip to content
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

Improving Jazzy Documentation for Multiple Modules #1369

Closed

Conversation

argjiramala-tomtom
Copy link
Contributor

@argjiramala-tomtom argjiramala-tomtom commented Oct 10, 2023

This pull request introduces enhancements to the Jazzy documentation tool, specifically focusing on supporting multiple modules within Xcode projects or workspaces.

To summarize the changes made:

  • New Command Line Attribute: A new command line attribute called "modules" has been added. This attribute allows users to specify multiple module names when generating documentation using Jazzy.
    Usage example:
    jazzy --modules ModuleName1,ModuleName2,...,ModuleNameN

  • Support for Workspace-based projects: If you have multiple Xcode projects that can only be successfully built using a workspace, follow these steps:

a. Each module should have its own configuration file with "xcodebuild_arguments" specified

Usage example:
jazzy --modules ModuleName1,ModuleName2,...,ModuleNameN

Proposal PR: #1363

Document multiple modules which are Xcode project on their own or part of a workspace
@johnfairh
Copy link
Collaborator

Thanks for the PR -- raises some interesting issues about CLI/config for multiple modules.

I don't understand the workspace problem you are trying to solve: the way it's described and coded seems very specific to your personal project/environment. Can you explain the problem you have?

On the design side:

We should have just one parameter --modules that can take a list of modules, with the default single-module mode falling out as a subset of that.

I think what the piece about workspace/scheme stuff reveals is that some CLI/config parameters need to be per-module scope rather than global. I can't think how to do that on the CLI so might have to be config-file only. So those per-module flags might include:

  • --build-tool-arguments (which would allow your scheme/workspace stuff I guess)
  • --source-directory (maybe - your example seems to need this but needs investigation!)
  • --swift-build-tool (probably not -- I think it's OK to say that if you're building multiple modules then they share this)
  • --sourcekitten-sourcefile (eg. mixed objc+swift modules)

...need to eventually go through them all.

* rename the command line attribute to modules
* use config file for module to build Xcode project
@argjiramala-tomtom
Copy link
Contributor Author

Thanks for the PR -- raises some interesting issues about CLI/config for multiple modules.

I don't understand the workspace problem you are trying to solve: the way it's described and coded seems very specific to your personal project/environment. Can you explain the problem you have?

On the design side:

We should have just one parameter --modules that can take a list of modules, with the default single-module mode falling out as a subset of that.

I think what the piece about workspace/scheme stuff reveals is that some CLI/config parameters need to be per-module scope rather than global. I can't think how to do that on the CLI so might have to be config-file only. So those per-module flags might include:

  • --build-tool-arguments (which would allow your scheme/workspace stuff I guess)
  • --source-directory (maybe - your example seems to need this but needs investigation!)
  • --swift-build-tool (probably not -- I think it's OK to say that if you're building multiple modules, then they share this)
  • --sourcekitten-sourcefile (eg. mixed objc+swift modules)

...need to eventually go through them all.

The problem with the workspace was when the project could be built only through the workspace by specifying the scheme. The issue was solved by using the jazzy config file of the module.
I have modified that part, where now when building the project it uses the module config file instead of the global config-file.

Also the renamed the parameter from: "--multiple_modules" to: "--modules"

@johnfairh
Copy link
Collaborator

I think there are two use cases for building multiple modules:

  1. All Jazzy needs is the module name, SPM/Xcodebuild will just work. This will be very common especially for SPM and the 'App + Extensions in one Xcode project' that users often ask about. This can work on the command line or in the config file. This could be a new --modules basically just like in the PR today.
  2. Jazzy needs special build flags for each module. This is needed for Xcode with workspaces and schemes and anything with more complicated build flags (and any Objective-C module in the future). I think this has to be config-file only, something like this where custom_modules is an array of objects whose keys are about the module:
custom_modules:
 - name: ModuleA
   build_tool_arguments: [ '-workspace', 'MyWorkspace', '-scheme', 'MySchemeA']
   source_directory: # for example, if this is necessary
   # any other per-module configuration
 - name: ModuleB
   build_tool_arguments:
    - -workspace
    - MyWorkSpace
    - -scheme
    - MySchemeB

Then there's some config code to write to look at these sources of modules, raise an error if the user has given multiple, and turn them into some single format for the rest of the code.

(I wrote custom_modules as an example name here because I don't have any good ideas and it goes with custom_categories which is the other major config-file only thing...)

def self.multiple_modules(options)
modules_parsed = Array[]
options.modules.each do |arguments|
module_parsed_string = Dir.chdir(arguments['source_directory']) do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be a bit careful using source_directory here - see the special treatment the top-level option gets in config.rb, needs full-path expansion and using pwd if the field's not set.

end

# Get from the remaining docs if there are extensions that should also be part of this module.
group = group.compact.map { |group|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get what this is doing; some issues/questions:

  1. Using a child decl to identify the declaring module is neat, but not all extensions have children (think extension SomeType : MyProtocol {}).
  2. When naming newdoc how about including the originating module name? So it would be MyModule+Swift.Int or something -- otherwise not sure how the user knows where the extended type is from. (Or even just Swift.Int given this is already nested under MyModule)
  3. I don't understand the scenario on line 137 where there is stuff left over that doesn't belong to any of the modules. What is an example of this?

About point 1, maybe think about changing how parse() works for multiple modules: could do each module's JSON one by one, then would always know which module is being worked on. At the moment group_docs_per_module is really just undoing work that's already been done to combine everything.

'in a custom category appear in generic groups at the ' \
'end. Example: https://git.io/v4Bcp',
config_attr :modules,
description: 'Array of modules that are going to be documented ' \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is unclear to me. I assume it's a list of modules, since array is an implementation concept, and they are not "being documented".
The line with the arguments puzzles me too, but maybe I need to dive into the tool more first.

raise 'Usage of config and comand line'
if modules_configured && module_name_configured
raise 'Jazzy only allows the use of a single command for generating documentation.' \
'Using both module configuration and modules configuration together is not supported.'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be a warning that we'll add the single module given to the list of modules and just proceed.

@johnfairh johnfairh mentioned this pull request Feb 8, 2024
10 tasks
@johnfairh
Copy link
Collaborator

Continued in #1379

@johnfairh johnfairh closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants