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

Add notebook document support #107

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

CS-Account
Copy link
Contributor

@CS-Account CS-Account commented Oct 23, 2024

Description of Changes

This change adds support for notebooks, such as those used by Jupyter Notebooks (NotebookDocument), by utilizing onDidSaveNotebookDocument as onDidSaveTextDocument is not triggered when a notebook is saved.

References to document.fileName have been replaced with document.uri.fsPath using a Union type to support both types, as NotebookDocument does not have the equivalent fileName attribute.

This PR also includes a small update to launch.json to disable extensions during development testing.

Closes #57

Testing

This extension does not currently have a test suite, but the functionality was tested manually for both text documents and notebook documents.

Testing Steps:

  • Create a notebook file and text file.
  • Use an application configuration such as:
    {
    	"emeraldwalk.runonsave": {
    		"commands": [
    			{
    				// Run whenever any file is saved
    				"match": ".*",
    				"cmd": "echo '${fileBasename}' saved."
    			}
    		]
    	}
    }
  • Save with both types and ensure correct output

Checklist

  • [✔️] Did you update any relevant docs / samples?
  • [✔️] Did you include details on how to test your changes?

src/extension.ts Outdated Show resolved Hide resolved
Copy link
Member

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Changes look good overall. A few things

  • Need to move the new Document type to the model.ts module.
  • No need to version bump. This will happen as part of release
  • I would suggest rebasing on latest main

package.json Outdated Show resolved Hide resolved
@CS-Account CS-Account force-pushed the add-notebook-document-support branch from 8cb6691 to f2e9878 Compare October 25, 2024 12:32
@CS-Account CS-Account force-pushed the add-notebook-document-support branch from f2e9878 to 14a4d3e Compare October 25, 2024 12:39
@CS-Account CS-Account requested a review from bmingles October 25, 2024 12:47
@bmingles
Copy link
Member

Looks good to me. I verified the behavior in notebook + regular file.

@bmingles bmingles closed this Oct 26, 2024
@bmingles bmingles reopened this Oct 26, 2024
@bmingles bmingles merged commit 96c6da5 into emeraldwalk:main Oct 26, 2024
@bmingles
Copy link
Member

Released in v0.2.7

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.

Feature request: Support actions on saving Jupyter Notebooks
2 participants