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

Redesign snippets workflow #225

Closed
0x009922 opened this issue Nov 11, 2022 · 1 comment · Fixed by #232
Closed

Redesign snippets workflow #225

0x009922 opened this issue Nov 11, 2022 · 1 comment · Fixed by #232

Comments

@0x009922
Copy link
Contributor

0x009922 commented Nov 11, 2022

While trying to solve hyperledger-iroha/iroha-javascript#130, I had to dive into details how snippets mechanism is currently designed (and implemented). I have some ideas that should simplify the documentation codebase (scripts part), make the whole approach more consistent with VitePress, and simplify an end-user (me) experience a bit.

My points are:

  • First of all, make use of VitePress' builtin <<< snippet syntax. Some examples to illustrate its power:

    Importing whole snippet as-is:
    
    <<<@/snippets/any.ts
    
    Highlight lines:
    
    <<<@/test.rs{1,3,5-7}
    
    Extract code regions:
    
    <<<@/test.rs#lorem
    
  • Use VSCode region syntax (// #region <name>) within source files instead of custom (// BEGIN FRAGMENT: <name>) syntax. It is:

    • supported by VitePress out of the box and doesn't require usage of dst-parser package (simpler code-base)
    • supported by a widely used IDE and maybe others too
  • Snippets downloading - just download them into src/snippets/ dir and leave their content as is, without parsing. It will make possible to not specify any code regions in a code file if it is not necessary.

  • snippet_sources.json:

    • remove version field
    • remove lang field - language is automatically inferred from the file extension. Even in the most advanced case when, for instance, the region within a Rust file effectively is an SQL, it could be solved with <<<@/sample.rs#sql-region{sql}
    • add optional name field for a file name the snippet will be written into within src/snippets/ dir. If it is omitted, then the name could be inferred from the URL.
  • Remove src/snippets/meta.json - it is unnecessary.

If generalise this issue, I propose 1) to achieve generalisation by actual simplification of the codebase and 2) to not reinvent the wheel.

@outoftardis
Copy link
Contributor

This approach LGTM

0x009922 added a commit that referenced this issue Dec 19, 2022
* [refactor]: import snippets in a new way

Signed-off-by: Dmitry Balashov <[email protected]>

* [misc]: update deps

Signed-off-by: Dmitry Balashov <[email protected]>

* [chore]: change sample http snippet

Signed-off-by: Dmitry Balashov <[email protected]>

* [feat]: enhance CLI, update scripts

Signed-off-by: Dmitry Balashov <[email protected]>

* [build]: clean deps a bit

Signed-off-by: Dmitry Balashov <[email protected]>

* [refactor]: bunch of changes

- exclude `/src/snippets` from `tsconfig`
- re-write Snippets guide
- cli: make `force` option enabled by default
- bump dependencies

Signed-off-by: Dmitry Balashov <[email protected]>

* Apply suggestions from code review

Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: 0x009922 <[email protected]>

* [docs]: apply changes from code review; chores

Signed-off-by: Dmitry Balashov <[email protected]>

* [revert]: return sample snippet back

Signed-off-by: Dmitry Balashov <[email protected]>

* Update src/documenting/snippets.md

Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: 0x009922 <[email protected]>

* Update etc/types.ts

Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: 0x009922 <[email protected]>

* [chore]: fix format

Signed-off-by: Dmitry Balashov <[email protected]>

* [ci]: update pr check

Signed-off-by: Dmitry Balashov <[email protected]>

Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: 0x009922 <[email protected]>
Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
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 a pull request may close this issue.

2 participants