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

CodeMirror implementation of GraphNodeComment #11585

Merged
merged 25 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
62d8bdd
Render tables in documentation.
kazcw Nov 6, 2024
40875ef
CHANGELOG, prettier
kazcw Nov 15, 2024
6bd2b95
Apply @farmaazon review.
kazcw Nov 15, 2024
c6432d0
Fix
kazcw Nov 15, 2024
be9bdd3
Lint
kazcw Nov 15, 2024
3d75871
Cleanup
kazcw Nov 15, 2024
e654dfd
Integration tests for GraphNodeComment
kazcw Nov 15, 2024
f317637
Merge remote-tracking branch 'origin/develop' into wip/kw/doc-tables
kazcw Nov 18, 2024
7431384
Workaround stuck CI
kazcw Nov 18, 2024
58adff3
Merge branch 'wip/kw/node-comment-tests' into staging
kazcw Nov 18, 2024
621be55
Merge branch 'wip/kw/doc-tables' into staging
kazcw Nov 18, 2024
89cf8df
CodeMirror implementation of GraphNodeComment
kazcw Nov 15, 2024
caaa01a
Revert "Workaround stuck CI"
kazcw Nov 18, 2024
4606f2b
Merge branch 'develop' into wip/kw/node-comment-tests
mergify[bot] Nov 18, 2024
4f8c80f
Merge branch 'develop' into wip/kw/doc-tables
mergify[bot] Nov 18, 2024
fcac2fd
Revert "Workaround stuck CI"
kazcw Nov 18, 2024
ae2b93f
Merge remote-tracking branch 'origin/wip/kw/node-comment-tests' into …
kazcw Nov 19, 2024
c58c8b7
Merge remote-tracking branch 'origin/develop' into wip/kw/doc-tables
kazcw Nov 19, 2024
fb396d8
Merge branch 'wip/kw/doc-tables' into wip/kw/cm-comments
kazcw Nov 19, 2024
b7549bc
Merge remote-tracking branch 'origin/develop' into wip/kw/doc-tables
kazcw Nov 19, 2024
f11dcce
Fix merge
kazcw Nov 19, 2024
2583a10
Merge branch 'wip/kw/doc-tables' into wip/kw/cm-comments
kazcw Nov 19, 2024
e80de7b
LINKABLE_URL_REGEX: Remove a Lexical special case
kazcw Nov 19, 2024
373c56c
Merge remote-tracking branch 'origin/develop' into wip/kw/cm-comments
kazcw Nov 20, 2024
d9f2c02
Space cases
kazcw Nov 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions app/gui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@
"@codemirror/view": "^6.28.3",
"@fast-check/vitest": "^0.0.8",
"@floating-ui/vue": "^1.0.6",
"@lexical/link": "^0.16.0",
"@lexical/plain-text": "^0.16.0",
"@lexical/utils": "^0.16.0",
"@lezer/common": "^1.1.0",
"@lezer/highlight": "^1.1.6",
"@noble/hashes": "^1.4.0",
Expand All @@ -107,7 +104,6 @@
"hash-sum": "^2.0.0",
"install": "^0.13.0",
"isomorphic-ws": "^5.0.0",
"lexical": "^0.16.0",
"lib0": "^0.2.85",
"magic-string": "^0.30.3",
"murmurhash": "^2.0.1",
Expand Down
3 changes: 2 additions & 1 deletion app/gui/src/dashboard/layouts/AssetDocs/AssetDocs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { AssetType } from '#/services/Backend'
import { useSuspenseQuery } from '@tanstack/react-query'
import { useCallback } from 'react'
import * as ast from 'ydoc-shared/ast'
import { normalizedMarkdownToStandard } from 'ydoc-shared/ast/documentation'
import { splitFileContents } from 'ydoc-shared/ensoFile'
import { versionContentQueryOptions } from '../AssetDiffView/useFetchVersionContent'

Expand Down Expand Up @@ -48,7 +49,7 @@ export function AssetDocsContent(props: AssetDocsContentProps) {

for (const statement of module.statements()) {
if (statement instanceof ast.MutableFunctionDef && statement.name.code() === 'main') {
return statement.documentationText() ?? ''
return normalizedMarkdownToStandard(statement.mutableDocumentationMarkdown().toJSON())
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 turn this into a json? We expect a md string to parse it and display

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The toJSON method of Y.Text returns the unformatted text content as a string.

}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<script setup lang="ts">
import PlainTextEditor from '@/components/PlainTextEditor.vue'
import { useAstDocumentation } from '@/composables/astDocumentation'
import { useFocusDelayed } from '@/composables/focus'
import { useGraphStore, type Node } from '@/stores/graph'
import { type Node } from '@/stores/graph'
import { nodeMutableDocumentation } from '@/util/ast/node'
import { syncRef } from '@vueuse/core'
import { computed, ref, type ComponentInstance } from 'vue'

Expand All @@ -12,34 +12,31 @@ const props = defineProps<{ node: Node }>()
const textEditor = ref<ComponentInstance<typeof PlainTextEditor>>()
const textEditorContent = computed(() => textEditor.value?.contentElement)

const graphStore = useGraphStore()
const { documentation } = useAstDocumentation(graphStore, () => props.node.outerAst)
const documentation = computed(() => nodeMutableDocumentation(props.node))

syncRef(editing, useFocusDelayed(textEditorContent).focused)
</script>
<template>
<div v-if="editing || documentation.state.value.trimStart()" class="GraphNodeComment">
<PlainTextEditor
ref="textEditor"
:modelValue="documentation.state.value"
@update:modelValue="documentation.set"
@keydown.enter.capture.stop="editing = false"
/>
<div
v-if="documentation && (editing || documentation.toJSON().trimStart())"
class="GraphNodeComment"
@keydown.enter.capture.stop="editing = false"
>
<PlainTextEditor ref="textEditor" :content="documentation" />
</div>
</template>

<style scoped>
.GraphNodeComment > :deep(.LexicalContent) {
:deep(.cm-content) {
display: inline-block;
padding: 0 8px 0 8px;
min-width: 22px;
border-radius: var(--radius-default);
background-color: var(--node-color-no-type);
color: var(--color-text-inversed);
font-weight: 400;
}

.GraphNodeComment :deep(code) {
color: var(--color-text-inversed);
:deep(.cm-line) {
padding: 0 8px 0 8px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from '@/components/GraphEditor/clipboard'
import { type Node } from '@/stores/graph'
import { Ast } from '@/util/ast'
import { nodeFromAst } from '@/util/ast/node'
import { nodeDocumentationText, nodeFromAst } from '@/util/ast/node'
import { Blob } from 'node:buffer'
import { expect, test } from 'vitest'
import { assertDefined } from 'ydoc-shared/util/assert'
Expand Down Expand Up @@ -82,8 +82,7 @@ test.each([...testNodes.map((node) => [node]), testNodes])(
const clipboardItem = clipboardItemFromTypes(nodesToClipboardData(sourceNodes))
const pastedNodes = await nodesFromClipboardContent([clipboardItem])
sourceNodes.forEach((sourceNode, i) => {
const documentation =
sourceNode.outerAst.isStatement() ? sourceNode.outerAst.documentationText() : undefined
const documentation = nodeDocumentationText(sourceNode) || undefined
expect(pastedNodes[i]?.documentation).toBe(documentation)
expect(pastedNodes[i]?.expression).toBe(sourceNode.innerExpr.code())
expect(pastedNodes[i]?.metadata?.colorOverride).toBe(sourceNode.colorOverride)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { NodeCreationOptions } from '@/composables/nodeCreation'
import type { GraphStore, Node, NodeId } from '@/stores/graph'
import { Ast } from '@/util/ast'
import { Pattern } from '@/util/ast/match'
import { nodeDocumentationText } from '@/util/ast/node'
import { Vec2 } from '@/util/data/vec2'
import type { ToValue } from '@/util/reactivity'
import * as iter from 'enso-common/src/utilities/data/iter'
Expand Down Expand Up @@ -186,10 +187,9 @@ export function writeClipboard(data: MimeData) {
// === Serializing nodes ===

function nodeStructuredData(node: Node): CopiedNode {
const documentation = node.outerAst.isStatement() ? node.outerAst.documentationText() : undefined
return {
expression: node.innerExpr.code(),
documentation,
documentation: nodeDocumentationText(node) || undefined,
metadata: node.rootExpr.serializeMetadata(),
...(node.pattern ? { binding: node.pattern.code() } : {}),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,28 @@ test.each([
},
],
},
{
markdown: '[Link text](<https://www.example.com/index.html>)',
expectedLinks: [
{
text: 'Link text',
href: 'https://www.example.com/index.html',
},
],
},
Comment on lines +67 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

You may consider adding cases for:

  1. Link with spaces and <>
  2. Link with spaces without <> (invalid case)
    Or we may assume that those cases in image are sufficient. But then I would consider moving them here, as "link" seems to me more basic that images.

{
markdown: '[Link text](<https://www.example.com/Url with spaces.html>)',
expectedLinks: [
{
text: 'Link text',
href: 'https://www.example.com/Url with spaces.html',
},
],
},
{
markdown: '[Link text](https://www.example.com/Spaces not allowed without angle brackets.html)',
expectedLinks: [],
},
{
markdown: '[Unclosed url](https://www.example.com/index.html',
expectedLinks: [],
Expand Down
56 changes: 13 additions & 43 deletions app/gui/src/project-view/components/PlainTextEditor.vue
Original file line number Diff line number Diff line change
@@ -1,52 +1,22 @@
<script setup lang="ts">
import FloatingSelectionMenu from '@/components/FloatingSelectionMenu.vue'
import { lexicalTheme, useLexical, type LexicalPlugin } from '@/components/lexical'
import LexicalContent from '@/components/lexical/LexicalContent.vue'
import { autoLinkPlugin, useLinkNode } from '@/components/lexical/LinkPlugin'
import LinkToolbar from '@/components/lexical/LinkToolbar.vue'
import { useLexicalStringSync } from '@/components/lexical/sync'
import { registerPlainText } from '@lexical/plain-text'
import { ref, useCssModule, watch, type ComponentInstance } from 'vue'
import { type ComponentInstance, computed, defineAsyncComponent, ref } from 'vue'
import * as Y from 'yjs'

const text = defineModel<string>({ required: true })
const props = defineProps<{ content: Y.Text | string }>()

const contentElement = ref<ComponentInstance<typeof LexicalContent>>()
const impl = ref<ComponentInstance<typeof LazyPlainTextEditor>>()

const plainText: LexicalPlugin = {
register: registerPlainText,
}
const LazyPlainTextEditor = defineAsyncComponent(
() => import('@/components/PlainTextEditor/PlainTextEditorImpl.vue'),
)

const textSync: LexicalPlugin = {
register: (editor) => {
const { content } = useLexicalStringSync(editor)
watch(text, (newContent) => content.set(newContent), { immediate: true })
watch(content.editedContent, (newContent) => (text.value = newContent))
},
}

const theme = lexicalTheme(useCssModule('lexicalTheme'))
const { editor } = useLexical(contentElement, 'PlainTextEditor', theme, [
autoLinkPlugin,
plainText,
textSync,
])
const { urlUnderCursor } = useLinkNode(editor)

defineExpose({ contentElement })
defineExpose({
contentElement: computed(() => impl.value?.contentElement),
})
</script>

<template>
<LexicalContent ref="contentElement" v-bind="$attrs" />
<FloatingSelectionMenu :selectionElement="contentElement">
<LinkToolbar v-if="urlUnderCursor" :url="urlUnderCursor" />
</FloatingSelectionMenu>
<Suspense>
<LazyPlainTextEditor ref="impl" v-bind="props" class="PlainTextEditor" />
</Suspense>
</template>

<style module="lexicalTheme">
.link {
color: #ddf;
&:hover {
text-decoration: underline;
}
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<script setup lang="ts">
import EditorRoot from '@/components/codemirror/EditorRoot.vue'
import { yCollab } from '@/components/codemirror/yCollab'
import { linkifyUrls } from '@/components/PlainTextEditor/linkifyUrls'
import { EditorState } from '@codemirror/state'
import { EditorView } from '@codemirror/view'
import { type ComponentInstance, computed, onMounted, ref, watchEffect } from 'vue'
import { Awareness } from 'y-protocols/awareness'
import { assert } from 'ydoc-shared/util/assert'
import * as Y from 'yjs'

const { content } = defineProps<{ content: Y.Text | string }>()

const editorRoot = ref<ComponentInstance<typeof EditorRoot>>()
const awareness = new Awareness(new Y.Doc())
const editorView = new EditorView()

function init(content: Y.Text | string) {
const baseExtensions = [linkifyUrls]
if (typeof content === 'string') {
return { doc: content, extensions: baseExtensions }
} else {
assert(content.doc !== null)
const yTextWithDoc: Y.Text & { doc: Y.Doc } = content as any
const doc = content.toString()
const syncExt = yCollab(yTextWithDoc, awareness)
return { doc, extensions: [...baseExtensions, syncExt] }
}
}

watchEffect(() => {
const { doc, extensions } = init(content)
editorView.setState(EditorState.create({ doc, extensions }))
})

onMounted(() => editorRoot.value?.rootElement?.prepend(editorView.dom))

defineExpose({
contentElement: computed(() => editorView.contentDOM),
})
</script>

<template>
<EditorRoot ref="editorRoot" />
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { linkifyUrls } from '@/components/PlainTextEditor/linkifyUrls'
import { EditorState } from '@codemirror/state'
import { Decoration, EditorView } from '@codemirror/view'
import { expect, test } from 'vitest'

function decorations<T>(
source: string,
recognize: (from: number, to: number, decoration: Decoration) => T | undefined,
) {
const state = EditorState.create({
doc: source,
extensions: [linkifyUrls],
})
const view = new EditorView({ state })
const decorationSets = state.facet(EditorView.decorations)
const results = []
for (const decorationSet of decorationSets) {
const resolvedDecorations =
decorationSet instanceof Function ? decorationSet(view) : decorationSet
const cursor = resolvedDecorations.iter()
while (cursor.value != null) {
const recognized = recognize(cursor.from, cursor.to, cursor.value)
if (recognized) results.push(recognized)
cursor.next()
}
}
return results
}

function links(source: string) {
return decorations(source, (from, to, deco) => {
if (deco.spec.tagName === 'a') {
return {
text: source.substring(from, to),
href: deco.spec.attributes.href,
}
}
})
}

// Test that link decorations are created for URLs and emails, with `href` set appropriately. The specific URL and email
// syntaxes recognized are tested separately, in the unit tests for `LINKABLE_URL_REGEX` and `LINKABLE_EMAIL_REGEX`.
test.each([
{
text: 'Url: https://www.example.com/index.html',
expectedLinks: [
{
text: 'https://www.example.com/index.html',
href: 'https://www.example.com/index.html',
},
],
},
{
text: 'Url: www.example.com',
expectedLinks: [
{
text: 'www.example.com',
href: 'https://www.example.com',
},
],
},
{
text: 'Email: [email protected]',
expectedLinks: [
{
text: '[email protected]',
href: 'mailto:[email protected]',
},
],
},
])('Link decoration: $text', ({ text, expectedLinks }) => {
expect(links(text)).toEqual(expectedLinks)
})
Loading
Loading