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

Render tables in documentation. #11564

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open

Render tables in documentation. #11564

wants to merge 17 commits into from

Conversation

kazcw
Copy link
Contributor

@kazcw kazcw commented Nov 15, 2024

Pull Request Description

Closes #9970.

Gravacao.de.Tela.2024-11-14.as.19.29.50.mov

Also:

  • Separate the parser for our flavor of Markdown from the CodeMirror integration; move the parser into ydoc-shared and use for Markdown line-wrapping.
  • Introduce our own version of yCollab extension; initially it is functionally equivalent to the upstream version, but translated to Typescript and our code style.
  • Refactor CodeEditor--better code organization and initialization logic.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

Also:
- Separate parser for our flavor of Markdown from the CodeMirror integration;
  move the parser into ydoc-shared and use for Markdown line-wrapping.
- Introduce our own version of yCollab extension; initially just the upstream
  version translated to Typescript and our code style.
- Refactor CodeEditor.
},
})

/** TODO: Add docs */
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's fill another missing docs here.

Comment on lines 22 to 23
watch([vueHost, toRef(props, 'source')], ([vueHost, source]) => {
if (!vueHost) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Since some version of Vue we can decompose properties

const { source } = defineProps<{ source: string}>

it makes source reactive, so there will be no need for toRef.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Props destructuring is actually a rather simplistic macro--it would be equivalent to replacing references to source with props.source. So the destructured bindings are reactive values, but they aren't reactive references that can be watched.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I didn't read the documentation to the end...

})

const editing = ref(false)
</script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we allow editing single cells? I see no logic for updating file after change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided during refinement not to allow cell edits until we have support for structural table edits. This is a simple synchronization implementation based on passing a string into the component; for edits we will replace it with a (local-only) cell YText that is synchronized with a slice of the whole-documentation YText.

Comment on lines 26 to 30
/**
* @param ytext
* @param awareness
* @param undoManager Set undoManager to false to disable the undo-redo plugin
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please write some summary here (we don't need empty @params)

If this is yCollab copy, write what changes we've made (and license information if required).

Comment on lines 170 to 175
) {
ast.module.transact(() => applyTextEditsToAstImpl(ast, textEdits, metadataSource))
}

function applyTextEditsToAstImpl(
ast: MutableAst,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we add new named function? We could just put it inside transact lambda.

@kazcw kazcw marked this pull request as draft November 15, 2024 17:55
@kazcw kazcw marked this pull request as ready for review November 15, 2024 17:56
Also a little refactoring in preparation for new implementation.
@farmaazon farmaazon self-requested a review November 18, 2024 16:27
@kazcw kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Nov 18, 2024
@kazcw kazcw added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Nov 18, 2024
@kazcw kazcw added the CI: Keep up to date Automatically update this PR to the latest develop. label Nov 18, 2024
@kazcw kazcw removed CI: Keep up to date Automatically update this PR to the latest develop. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Nov 19, 2024
@kazcw kazcw added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Nov 19, 2024
@kazcw kazcw mentioned this pull request Nov 19, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add table display support to documentation editor
2 participants