-
Notifications
You must be signed in to change notification settings - Fork 2
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 auto-hide options and other fixes #1
Add auto-hide options and other fixes #1
Conversation
I like this because it lets me press Command-S in VS Code and have it auto-format the code nicely. It might be controversial though when other people prefer different auto-formatters.
This solves the problem that the variable could run out of sync when hiding the lazygit window by manually activating a different editor tab; in this case the extension would still believe that lazygit was active. We could solve this by implementing onDidChangeActiveTextEditor() to catch this, but why: if we can determine our visible state without having to remember it, that's better.
import * as vscode from "vscode"; | ||
import * as fs from "fs"; | ||
import * as os from "os"; | ||
import { exec } from "child_process"; |
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.
I realize that formatting is always a matter of taste; I'm happy to drop this commit if you don't like it.
lgtm, thanks! |
I was a bit hasty -- without the isLazyGitVisible toggling isn't working for me correctly. I'll have a look at this more and clean up my code tomorrow |
toggling-lg.mp4 |
I would be curious under what conditions it doesn't work for you, can you describe a scenario? I'm not sure if the video that you posted is supposed to demonstrate this, but I can't quite see what you mean.
I feel there must be a misunderstanding here. I wasn't trying to report or fix a bug here, I was trying to implement a feature that I want to have. I can open a new PR for that once we sorted out the isLazyGitVisible thing. |
ah I misread here. there was a bug in a previous version where toggling lazygit would also toggle the terminal and side bar. you can disregard my previous comment, I'll have a better look at the PR tomorrow. |
Just to explain again what bug I was trying to fix with the removal of the isLazyGitVisible variable:
This is just one example of where things run out of sync (but a very common one for me), there are many others. My hope was that we could fix this whole class of problems by not keeping track of whether lazygit is visible, but instead determine it dynamically at the moment we need to know. Again, I'd be interested in scenarios where this didn't work correctly for you. |
done, |
Nice, this is a big improvement. I opened a new PR for the auto-hide feature: #2 |
lazygitPath
config show up in the settings view