-
Notifications
You must be signed in to change notification settings - Fork 35
Status bar icon toggle #91
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
7d6beb8
59a23aa
f15b55e
267ca0a
c9643c2
4a05dcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,18 +4,41 @@ import { registerCommands } from "./commands"; | |
| import config from "./config"; | ||
| import { getGitApi, GitAPI, RefType } from "./git"; | ||
| import { store } from "./store"; | ||
| import { commit, watchForChanges } from "./watcher"; | ||
| import { commit, watchForChanges, ensureStatusBarItem, updateStatusBarItem } from "./watcher"; | ||
| import { updateContext } from "./utils"; | ||
| import * as minimatch from "minimatch"; | ||
|
|
||
| // Helper function for file pattern matching | ||
| function matches(uri: vscode.Uri) { | ||
| return minimatch(uri.path, config.filePattern, { dot: true }); | ||
| } | ||
|
|
||
| export async function activate(context: vscode.ExtensionContext) { | ||
| // Wait for Git extension to be ready | ||
| const git = await getGitApi(); | ||
| if (!git) { | ||
| return; | ||
| } | ||
|
|
||
| // Initialize the store based on the | ||
| // user/workspace configuration. | ||
| store.enabled = config.enabled; | ||
| // Wait for initial repository to be available | ||
| if (git.repositories.length === 0) { | ||
| await new Promise<void>((resolve) => { | ||
| const disposable = git.onDidOpenRepository(() => { | ||
| disposable.dispose(); | ||
| resolve(); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| // Initialize the store and context based on the configuration | ||
| const initialEnabled = vscode.workspace.getConfiguration('gitdoc').get('enabled', false); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous code was handling this logic already, since "config.enabled" pulls its value from the workspace configuration. Is there a reason we need to change it to this more verbose implementation? |
||
| store.enabled = initialEnabled; | ||
| updateContext(initialEnabled, false); // Set initial context to match config | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updateContext will already be run in response to setting store.enabled, so we don't need this. And then related to my previous comment, I think we can just revert this entire hunk. |
||
|
|
||
| // Create status bar item and show it immediately | ||
| const statusBar = ensureStatusBarItem(); | ||
| statusBar.show(); | ||
| context.subscriptions.push(statusBar); | ||
|
|
||
| registerCommands(context); | ||
|
|
||
|
|
@@ -25,9 +48,38 @@ export async function activate(context: vscode.ExtensionContext) { | |
| context.subscriptions.push(git.onDidOpenRepository(() => checkEnabled(git))); | ||
| context.subscriptions.push(git.onDidCloseRepository(() => checkEnabled(git))); | ||
|
|
||
| // Create a debounced version of updateStatusBarItem | ||
| let updateTimeout: NodeJS.Timeout | null = null; | ||
| const debouncedUpdateStatusBar = (editor: vscode.TextEditor | undefined) => { | ||
|
tomglynch marked this conversation as resolved.
Outdated
|
||
| if (updateTimeout) { | ||
| clearTimeout(updateTimeout); | ||
| } | ||
| updateTimeout = setTimeout(() => { | ||
| updateStatusBarItem(editor); | ||
| }, 50); // 50ms debounce | ||
| }; | ||
|
|
||
| // Watch for active editor changes to update icon state | ||
| context.subscriptions.push( | ||
| vscode.window.onDidChangeActiveTextEditor((editor) => { | ||
| debouncedUpdateStatusBar(editor); | ||
| }) | ||
| ); | ||
|
|
||
| // Set initial icon state | ||
| updateStatusBarItem(vscode.window.activeTextEditor); | ||
|
|
||
| // Initial check of enabled state | ||
| await checkEnabled(git); | ||
|
|
||
| // Watch for store changes | ||
| reaction( | ||
| () => [store.enabled], | ||
| () => checkEnabled(git) | ||
| () => store.enabled, | ||
| (enabled) => { | ||
| updateContext(enabled, true); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't need this since checkEnabled will already update the context as needed. |
||
| checkEnabled(git); | ||
| debouncedUpdateStatusBar(vscode.window.activeTextEditor); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be handled by the checkEnabled function, and effectively revert to the previous code. That way this reaction is simply watching for enabled changes and then updating the UX based on that (via checkEnabled)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, you already have a reaction in the watcher module that will update the status bar when the store.enabled changes. So I think we can just remove this? |
||
| } | ||
| ); | ||
|
|
||
| context.subscriptions.push( | ||
|
|
@@ -37,6 +89,7 @@ export async function activate(context: vscode.ExtensionContext) { | |
| e.affectsConfiguration("gitdoc.autoCommitDelay") || | ||
| e.affectsConfiguration("gitdoc.filePattern")) { | ||
| checkEnabled(git); | ||
| debouncedUpdateStatusBar(vscode.window.activeTextEditor); | ||
|
tomglynch marked this conversation as resolved.
Outdated
|
||
| } | ||
| }) | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicating this function, we should define it as a reusable utility that can be imported in this file and in the watcher.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just export the function from the watcher module and then reuse it here