-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
rework scm, recover git extension #5416
Conversation
@@ -98,6 +98,13 @@ | |||
"@phosphor/signaling" "^1.2.2" | |||
"@phosphor/virtualdom" "^1.1.2" | |||
|
|||
"@primer/octicons-react@^9.0.0": |
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.
@marcdumais-work Could you please run your tool on new deps to check whether everything is alright? 🙏
We hope to test and merge today/tomorrow and then release 0.7.2.
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.
@akosyakov I did the "deep" check, recursively covering all production dependencies for the whole repo/PR, and everything looks good.
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.
@marcdumais-work thank you
Theia git extension is ready for review. I will add some change for VS Code extension to fix triggering commit from the keyboard and improve decorations, but they should not have significant impact. |
looking into failing tests... |
Looking into it now. |
will do, but it was not highlighted blue before 😛 you can check in Gitpod prod |
First off, it works reliably, I cannot see the repository watcher starting for each file I open in the editor 👍 I collected a few things, none of them are critical. The decorator behavior is odd, however.
|
Steps to reproduce:
|
We have to document it. Please add one single sentence about the change to the doc: |
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 have tested this and it works very well. Only read through 50% of the code so far but that was very good, too. thank you!
@@ -14,6 +14,8 @@ | |||
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 | |||
********************************************************************************/ | |||
|
|||
// tslint:disable:no-any |
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.
[minor]: I would prefer having three distinct no-any
instead of disabling this rule for the entire file.
Just shimming in as I see the comments and approvals already, but CI is failing because of missing bindings in the tests. |
Works pretty well @akosyakov! Small issue with |
@kittaakos Do you know hot to achieve it? I am using kebinding service to get an accelerator. It seems to be the generic issue. |
I have opened a follow-up: #5427. |
@AlexTugarev Can you comment it? Was it always the case? git-diff module was not touched at all by scm changes. |
It works differently with the " If I have two repositories, |
|
8fc6f8d
to
8379164
Compare
Input field should be highlighted as focused now when selected and should not flicker anymore. cc @svenefftinge @kittaakos |
@kittaakos Would it be fine to tackle #5416 (comment) separately? Could you file an issue please? please point which repo you have used, i've tried with monaco-languageclient and could not reproduce |
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.
Thanks for taking care of this Anton!
@akosyakov I tested with the "commit 8379164" |
@@ -102,6 +102,10 @@ | |||
padding: 4px 4px 4px 4px; | |||
} | |||
|
|||
.theia-scm-input-message-container textarea[placeholder] { | |||
text-overflow: ellipsis; |
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.
This does not seem to work in Chrome but FF :/ I will rework it.
It works now. I have verified the behavior with the default layout in Safari, FF, and Chrome.
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
+ align view options handling with VS Code + fix preserving focus for documents opened by plugins Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
… to align with VS Code e.g. diff URIs should be unwrapped Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Aligned the `p-debounce` version between `@theia/debug` and `@theia/scm` Aligned styles (`padding`) with VS Code. Got rid of the `messageBoxHeight` property. Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Otherwise, the placeholder won't fit into the default side-panel width and multi line `textarea` will be rendered instead. This CSS style adjusts the `textarea` behavior to VS Code. Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@akosyakov, I am going to force push to this branch. Sorry :( |
@kittaakos it is completely fine! Pease merge if you are fine with the PR! |
I did another cycle; It worked nicely ✨ Thank you for the fix! |
Thanks for all testing! |
Now that `octicons` are supported in the statusbar thanks to #5416, the icon for `running tasks...` is now updated to match that of its vscode counterpart. Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
Now that `octicons` are supported in the statusbar thanks to #5416, the icon for `running tasks...` is now updated to match that of its vscode counterpart. Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
Generally for this PR
Things to test:
Fixed issues
when
closuresUI changes for Theia git extension to align with VS Code
Commit
button anymore but a check in the toolbar:- git diff editors have shorter titles instead of using `lalala (abc123) <-> lalala (HEAD)`
Let me know if you prefer to put
Commit
button back (we will need to make it generally then for VS Code git extension as well) and have more verbose diff editor titles.Theia Git extension UI
VS Code Git extension UI