Skip to content

[Docs] Extract example php code for static analysis.#5689

Merged
kenjis merged 5 commits intocodeigniter4:developfrom
sfadschm:docs-example-code
Feb 22, 2022
Merged

[Docs] Extract example php code for static analysis.#5689
kenjis merged 5 commits intocodeigniter4:developfrom
sfadschm:docs-example-code

Conversation

@sfadschm
Copy link
Copy Markdown
Contributor

@sfadschm sfadschm commented Feb 13, 2022

This is the working PR for extracting php code examples from the docs to enable running php-cs-fixer on them.

All files will have consecutive numeric file names per subfolder (module).

This PR will not do any change to the output of the docs html build.
Content and style changes will follow with a second PR to ease review.

Workflow

  1. Manually mark all appearances of php code in an rst.
  2. Extract code via script.
  3. Run make html and diff new docs with base branch.
  4. Fix discrepancies from diff.

Status

  • Extract examples from all modules.
  • Remove empty lines.
  • Empty line at file ending.
  • Unify line endings.
  • Revert unnecessary examples.
  • Check if mixed HTML/inline PHP works in php-cs-fixer (it does)
  • Resolve merge conflicts.
  • Revert changes to make and conf.py.

Follow-up PR (content/style changes)

  • Replace utf-8 characters.
  • Remove occurences of . :.
  • Convert non-php code.
  • Display opening tags (for PHP) and closing tags (for mixed HTML).
  • Enable and run php-cs-fixer for user_guide_src\source.
  • Resolve merge conflicts.

Miscellaneous (possible stuff for the second PR)

  • There are some 'fake' lists and indentation that should be fixed, e.g.:
    • database -> metadata -> l. 53
    • database -> query-builder -> l. 295
    • database -> query-builder -> l. 238 (codeblock is not part of the lists)

Happy about feedback and discussion.

@sfadschm sfadschm changed the title [Docs] Extraxt example php code for static analysis. [Docs] Extract example php code for static analysis. Feb 13, 2022
@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 13, 2022

How about like this? Make the filename and folder name same.

cli/cli_commands.rst
cli/cli_commands/AppInfo.php
cli/cli_commands/call.php

@sfadschm
Copy link
Copy Markdown
Contributor Author

How about like this? Make the filename and folder name same.

cli/cli_commands.rst
cli/cli_commands/AppInfo.php
cli/cli_commands/call.php

You mean, sparing the extra folder level examples?
Inside this folder, the structure is like you mentioned.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 13, 2022

You mean, sparing the extra folder level examples?

Yes. I thought it was better the RST file and source file are close, but the difference might be not so much.

Inside this folder, the structure is like you mentioned.

A bit different.
At least, I expect the exact same name.

cli/
├── cli.rst
├── cli_commands.rst
├── cli_generators.rst
├── cli_library.rst
├── cli_request.rst
└── examples
    ├── cli
    ├── cli_commands
    ├── cli_generators
    └── cli_request

@kenjis kenjis added the documentation Pull requests for documentation only label Feb 14, 2022
@sfadschm
Copy link
Copy Markdown
Contributor Author

A bit different.

Oh right, I get it now. cli was off as this was the first folder that I did manually.

@sfadschm
Copy link
Copy Markdown
Contributor Author

Yes. I thought it was better the RST file and source file are close, but the difference might be not so much.

Alright, I will leave this for a second opinion, but I am fine both ways.

Comment thread user_guide_src/source/cli/examples/cli/routes.php Outdated
@paulbalandan
Copy link
Copy Markdown
Member

I don't have a strong opinion on where to place the code samples. Where it fits then that's fine.

My question: is the extraction manually done? At least in the description, it says:

Extract code via script.

So I'm looking in the commits for a script that does that. Or maybe I got lost because of the many files changed?

@sfadschm
Copy link
Copy Markdown
Contributor Author

Oh sorry, what I do is that I manually mark the beginning and end of each php sample in a folder with a [xyz] tag. Then I run a local php script that preg-matches these tags, stores the sample to a php file and replaces it in the rst with the literalinclude directive.
If the sample contains no opening tag, the script adds this to the php file and adds the :lines: directive to the include.

Then I build the docs to another local repository, where I havr stored the original docs build before this PR and fix any differences by hand (mostly indentation and such ).

Unfortunately , I could not figure a way to identify php code samples automatically, as it is only introduced by :: like any other codeblock.

@sfadschm
Copy link
Copy Markdown
Contributor Author

Or maybe I got lost because of the many files changed?

Good point though, I will dedicate this PR to the pure extraction of the samples.

The cs-fixer run will then be another PR so the changes are reviewable.

@sfadschm
Copy link
Copy Markdown
Contributor Author

What are our line ending conventions?
LF for php files and CRLF for rst?

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 16, 2022

LF for all files.

@sfadschm
Copy link
Copy Markdown
Contributor Author

So the main work of this PR is done. Questions remaining for now:

  1. Do we stick to numeric file names?
  2. Can php-cs-fixer handle mixed html/php code? (I think not, so I spared such examples.)

Once these are done I will squash all commits and start the second PR.

@sfadschm
Copy link
Copy Markdown
Contributor Author

And a bigger question:
3. Should all php examples in the build start with <? php? This would allow us to have combined php/html examples by also including the ? > tag.

@paulbalandan
Copy link
Copy Markdown
Member

  1. Do we stick to numeric file names?

Personally, I would like numeric file names. Less thinking of file names. But I want it to be padded with leading zeros so that it will align nicely in file explorer. 01.php, 02.php, etc.

  1. Can php-cs-fixer handle mixed html/php code? (I think not, so I spared such examples.)

There are some rules which can handle mixed html/php code.

  1. Should all php examples in the build start with <? php? This would allow us to have combined php/html examples by also including the ? > tag.

If the code is pure php, then yes to opening tag. If mixed with html, then include the closing tag where necessary

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 18, 2022

This PR is really big!
+14,225 −9,320

How could we review?
Compare the output HTML?

@sfadschm
Copy link
Copy Markdown
Contributor Author

  1. Do we stick to numeric file names?

Personally, I would like numeric file names. Less thinking of file names. But I want it to be padded with leading zeros so that it will align nicely in file explorer. 01.php, 02.php, etc.

  1. Can php-cs-fixer handle mixed html/php code? (I think not, so I spared such examples.)

There are some rules which can handle mixed html/php code.

  1. Should all php examples in the build start with <? php? This would allow us to have combined php/html examples by also including the ? > tag.

If the code is pure php, then yes to opening tag. If mixed with html, then include the closing tag where necessary

I would agree with that.

@sfadschm
Copy link
Copy Markdown
Contributor Author

This PR is really big!
+14,225 −9,320

How could we review?
Compare the output HTML?

Correct, that is how I am doing it during the changes and why I removed the date from the HTML build.

This PR will not change the HTML output in any way.

So the way to go might be:

  1. Create a local copy of the base branch of this PR.
  2. Make git follow the build folder.
  3. Build the original docs and commit.
  4. Apply this PR.
  5. Build the new docs and see if git sees any diff.

Currently, I created a new local repository where I stored just the old build and then changed the make script to redirect new builds there.

If that helps I can also upload the original and new HTML, but this might go against the idea of independent review.

@sfadschm
Copy link
Copy Markdown
Contributor Author

Yes. I thought it was better the RST file and source file are close, but the difference might be not so much.

Alright, I will leave this for a second opinion, but I am fine both ways.

I will go with this. Flat folder structure makes it easier to work with both rst and php files.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 18, 2022

This PR will not change the HTML output in any way.

Great!

@sfadschm sfadschm marked this pull request as ready for review February 19, 2022 07:57
@sfadschm
Copy link
Copy Markdown
Contributor Author

This should now be up to date with develop.

@sfadschm
Copy link
Copy Markdown
Contributor Author

I will add a script to automatically restore consecutive enumeration of the examples.
Would a php script be fine for that cause? I am not really experienced with shell scripts for manipulating text files.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 19, 2022

Would a php script be fine for that cause? I am not really experienced with shell scripts for manipulating text files.

No problem.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 19, 2022

This should now be up to date with develop.

It seems this is a bit behind.

@sfadschm
Copy link
Copy Markdown
Contributor Author

This should now be up to date with develop.

It seems this is a bit behind.

Yep, since #5709 😆
I will do the script first so other changes do not get in the way again.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 19, 2022

I made html on develop and this branch.

$ diff -urN html.develop html | grep ^+++
+++ html/incoming/incomingrequest.html	2022-02-19 22:34:47.000000000 +0900
+++ html/installation/index.html	2022-02-19 22:34:50.000000000 +0900
+++ html/installation/upgrade_418.html	2022-02-19 22:34:56.000000000 +0900
+++ html/installation/upgrade_420.html	1970-01-01 09:00:00.000000000 +0900
+++ html/installation/upgrading.html	2022-02-19 22:35:01.000000000 +0900
+++ html/libraries/uploaded_files.html	2022-02-19 22:35:08.000000000 +0900
+++ html/searchindex.js	2022-02-19 22:35:18.000000000 +0900

@sfadschm
Copy link
Copy Markdown
Contributor Author

Up tp date with e1b4f76.

The renumerate script is far from optimized, but I could not think of another logical approach.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Feb 22, 2022

I confirmed the generated HTML are the exact same as the develop 77c5ba9.

$ diff -urN html.develp/ html
$

@sfadschm Can I merge this PR?

@sfadschm
Copy link
Copy Markdown
Contributor Author

I think yes.
The fixes will follow up, but the longer we leave this open, the more conflicts we get :-D

@kenjis kenjis merged commit 661cf16 into codeigniter4:develop Feb 22, 2022
@sfadschm sfadschm deleted the docs-example-code branch February 22, 2022 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Pull requests for documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants