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

fix dashIndexFilePath in Info.plist #3

Open
mundanevision20 opened this issue Dec 12, 2022 · 2 comments
Open

fix dashIndexFilePath in Info.plist #3

mundanevision20 opened this issue Dec 12, 2022 · 2 comments

Comments

@mundanevision20
Copy link

mundanevision20 commented Dec 12, 2022

Hi @roberth-k,

Thanks for maintaining this. This docset has been very helpful to me so far.

It looks like the current value index.html of dashIndexFilePath is invalid as the file can not be located within the docsets Documents directory.

Using terraform/intro/index.html might be a good idea.

Off topic Three more things I noticed so far when using the docset:

  • I also noticed that the docset contains the original absolute links (e.g. https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace) instead of relative links (e.g. providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace) in references of provider docs. I understand that render.py seems to handle the conversion of relative links in line 281. Embedding a method to filter for provider docs contained with the docset would be great.

  • some docs for providers like e.g. Databricks are apparently using so called cuddled lists which require the cuddled-lists extension to be rendered with markdown2
    I propose to add the cuddled lists extension to the list of extensions in render.py

    # snippet from render.py - line 111
    # markdown2 extensions used as of writing
    extras = [
          'admonitions',
          'fenced-code-blocks',
          'header-ids',
          'markdown-in-html',
          'tables',
          # add 'cuddled-lists' here
      ]
    click to collapse before and after results
    <!-- source of this snippet: https://github.com/databricks/terraform-provider-databricks/blob/d4915ddc27a4f17b724ae02f0bb31540ebf73901/docs/index.md -->
    Compute resources
    * Deploy [databricks_cluster](resources/cluster.md) on selected [databricks_node_type](data-sources/node_type.md)
    * Schedule automated [databricks_job](resources/job.md)
    * Control cost and data access with [databricks_cluster_policy](resources/cluster_policy.md)
    * Speedup job & cluster startup with [databricks_instance_pool](resources/instance_pool.md)
    * Customize clusters with [databricks_global_init_script](resources/global_init_script.md)
    * Manage few [databricks_notebook](resources/notebook.md), and even [list them](data-sources/notebook_paths.md)
    * Manage [databricks_repo](resources/repo.md)
    <!-- before adding 'cuddled-lists' -->
    <p>Compute resources
    * Deploy <a href="resources/cluster.md">databricks<em>cluster</a> on selected <a href="data-sources/node_type.md">databricks</em>node<em>type</a>
    * Schedule automated <a href="resources/job.md">databricks</em>job</a>
    * Control cost and data access with <a href="resources/cluster_policy.md">databricks<em>cluster</em>policy</a>
    * Speedup job &amp; cluster startup with <a href="resources/instance_pool.md">databricks<em>instance</em>pool</a>
    * Customize clusters with <a href="resources/global_init_script.md">databricks<em>global</em>init<em>script</a>
    * Manage few <a href="resources/notebook.md">databricks</em>notebook</a>, and even <a href="data-sources/notebook_paths.md">list them</a>
    * Manage <a href="resources/repo.md">databricks_repo</a></p>
    <!-- after adding 'cuddled-lists' -->
    <p>Compute resources</p>
    
    <ul>
    <li>Deploy <a href="resources/cluster.md">databricks<em>cluster</a> on selected <a href="data-sources/node_type.md">databricks</em>node_type</a></li>
    <li>Schedule automated <a href="resources/job.md">databricks_job</a></li>
    <li>Control cost and data access with <a href="resources/cluster_policy.md">databricks<em>cluster</em>policy</a></li>
    <li>Speedup job &amp; cluster startup with <a href="resources/instance_pool.md">databricks<em>instance</em>pool</a></li>
    <li>Customize clusters with <a href="resources/global_init_script.md">databricks<em>global</em>init_script</a></li>
    <li>Manage few <a href="resources/notebook.md">databricks_notebook</a>, and even <a href="data-sources/notebook_paths.md">list them</a></li>
    <li>Manage <a href="resources/repo.md">databricks_repo</a></li>
    </ul>
  • minifying the assets (e.g. when returning html in line 236 of render.py might reduce the required disk quota and increase the performance. However this is not a issue from my point of view it's rather a nice to have.

I hope my comments are helpful to you. Unfortunately English is not my mother tongue. I'd like to apologize for any misleading or inappropriate language.

Looking forward to your feedback.

Kind regards,
Henning

@mundanevision20
Copy link
Author

mundanevision20 commented Dec 12, 2022

Regarding the conversion of the absolute links for providers contained in the docset the following approach might work:

  1. adding an additional property for the src/ directory

    @property
    def source_dir(self) -> str:
    	return str(Path(self.docset_dir).parent / "src")
  2. adding the 'conversion' of the path of the urls in update_hrefs()

    if path.startswith("https://registry.terraform.io/providers/") or path.startswith("https://www.terraform.io/docs/"):
    	path_temp = str(path).replace('https://registry.terraform.io/','')
    	path_temp = str(path_temp).replace('https://www.terraform.io/docs/','')
    
    	if '/' in path_temp:
    		path_parts = path_temp.replace('providers/','').split('/')
    		if len(path_parts) > 1:
    			provider_source = f"{args.source_dir}/{path_parts[0]}/{path_parts[1]}"
    			path = join(relpath('.', provider_source), path_temp)
    			if isdir(path):
    		path = path + '/index.html'
    			elif not path.endswith('.html'):
    		path = path + '.html'
    			print(f"  modified path: {path} \n")

This should create relative paths for all locally available files.

@roberth-k
Copy link
Owner

Hi @mundanevision20 --

Thank you for this very thorough report!

I've updated the index page and added the cuddled-lists extensions as you suggested; however, I'm not quite confident about replacing the absolute URL-s at this time, for the following reasons:

  • It would require checking that a given page is, indeed, in the docset (as not every provider is included).
  • The registry uses normalised paths that differ from those of provider sources, and there's a chance it further accommodates for legacy routes by way of redirects.

A compromise could be rewriting the links only after the rendering is complete, as then os.path.exists() would indicate whether a link is resolvable locally. I don't have capacity to focus on this in the near future, but maybe I'll consider it in the coming year.

Enjoy the festive season!

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

No branches or pull requests

2 participants