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

Feat: Add basic embedded support #38

Merged
merged 14 commits into from
Nov 3, 2023
Merged

Conversation

idillon-sfl
Copy link
Member

  • Add basic hover and completion support for Bash and Python
  • Save embedded documents on disk (we would rather not do it)
  • Delete embedded documents on original document deletion

server/src/tree-sitter/declarations.ts Show resolved Hide resolved
server/src/embedded-languages/python-support.ts Outdated Show resolved Hide resolved
server/src/embedded-languages/bash-support.ts Outdated Show resolved Hide resolved
server/src/embedded-languages/bash-support.ts Show resolved Hide resolved
server/src/embedded-languages/documents-manager.ts Outdated Show resolved Hide resolved
server/src/server.ts Outdated Show resolved Hide resolved
@WilsonZiweiWang
Copy link
Collaborator

It would be nice if you could add some logger.debug() in your functions and wrap some fs related function in try-catch. (You could skip the ones in the tests)

@idillon-sfl
Copy link
Member Author

It would be nice if you could add some logger.debug() in your functions and wrap some fs related function in try-catch. (You could skip the ones in the tests)

Good idea. I dit it

const originalPath = originalUriString.replace('file://', '')
const pathToEmbeddedDocumentsFolder = this.getPathToEmbeddedDocumentFolder(originalPath)
fs.mkdirSync(pathToEmbeddedDocumentsFolder, { recursive: true })
const randomName = randomUUID()
Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure, you always use the same random file for each language document, or is a new random file generated each parsing?

fs.mkdirSync(pathToEmbeddedDocumentsFolder, { recursive: true })
fs.writeFileSync(pathToEmbeddedDocument, embeddedDocumentContent)
} catch (error) {
logger.error('Failed to create embedded document:', error)
Copy link
Member

Choose a reason for hiding this comment

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

don't you want to exit the function or stop what you are doing at this point?

@WilsonZiweiWang
Copy link
Collaborator

I think it would be nice if you could try to break down some commits a little bit since some commit messages don't completely cover every change under it.

@WilsonZiweiWang
Copy link
Collaborator

Be sure to handle inline-python as well:
image

@idillon-sfl
Copy link
Member Author

I am not forgetting about inline-python. Just not sure yet how to do it (but I have some ideas).

I am now using promises for all the file system operations (create file, delete file, create directory, delete directory). I noticed the file deletion was async and it was giving inconsistent results for my tests.

@idillon-sfl idillon-sfl force-pushed the add-embedded-support branch 2 times, most recently from 2324bb0 to dff3c2e Compare November 2, 2023 00:29
@idillon-sfl idillon-sfl merged commit da8e612 into staging Nov 3, 2023
1 check passed
@idillon-sfl idillon-sfl deleted the add-embedded-support branch November 3, 2023 13:33
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 this pull request may close these issues.

3 participants