Summary:
It uses `tr` which is missing in `debugruntest` stdlib.
Let's just replace both `tr` and `jq` with a Python function to make it
debugruntest compatible.
Reviewed By: zzl0
Differential Revision: D42826353
fbshipit-source-id: e9e7d2da6d7bbf256d83050abb0ca2b58f5516f1
Summary:
If the commit is not a stack, the regular GitHub PR workflow works fine.
Skip reviewstack and stack boilerplate.
Reviewed By: sggutier
Differential Revision: D43481900
fbshipit-source-id: a7b6f93e1b6ac4de16469980b85c0c8261d5efee
Summary: This makes it auto-fix friendly.
Reviewed By: sggutier
Differential Revision: D43481899
fbshipit-source-id: e7c54980f1aac667f7e4cebb04ec420045db1655
Summary:
In order for the command to show up in our help text, we must add a stub and print out "not-implemented" if someone somehow manages to invoke the Python version of `prefetch-profile`.
Instead of adding stubs for each sub-subcommand for prefetch-profile, we'll simply redirect the user to the Rust help text.
Reviewed By: fanzeyi
Differential Revision: D43482266
fbshipit-source-id: f29d511bfb06a6d1cc74386cc88e685024384ece
Summary:
This was requested in https://github.com/facebook/sapling/issues/531.
We should let you click to open the diff view on changed files in the vscode SCM sidebar. We already support clicking to open a file. The diff view command is already implemented, so we just need to call it and handle SourceControlResourceStates.
Reviewed By: quark-zju
Differential Revision: D43477399
fbshipit-source-id: a3148ce1367778206139b3c8fdfe69a412fa5fbd
Summary:
Add a platform hook to open files in a platform-native diff view.
If this platform API is implemented, then we'll show a diff view icon next to each uncommitted change.
We currently only show this on the uncommitted changes section (not changes on each commit you select)
This API requires passing a specific comparison. Even though we only use uncommitted changes right now, this will let us use this feature elsewhere in the future.
Reviewed By: quark-zju
Differential Revision: D43468032
fbshipit-source-id: 8406f2821c2e304b9066a8a3de83941f1e065557
Summary:
We shouldn't let you commit/amend/submit with an image upload ongoing. Otherwise you'll get the placeholder text. We should tell you to wait until it finishes. If you're impatient and don't need the image you can click "cancel" to cancel all uploads.
We also want to have a tooltip explaining why it's disabled, otherwise it's a very confusing experience.
Reviewed By: quark-zju
Differential Revision: D43412102
fbshipit-source-id: 3236529945381f1d15ee9b675e338d13a449e3c0
Summary: If your image upload fails, we should let you know rather than just hiding the placeholder. We can do this with an inline error badge next to where the upload button is. Hovering shows the full error that was encountered. There's also a dismiss button to hide the error if you don't care about it.
Reviewed By: quark-zju
Differential Revision: D43411126
fbshipit-source-id: 1d6f029fd623c8dbcb54209a8e9dcb575c420bdd
Summary:
Add a new component to render short inline error messages. In general, I think it's nice to show contextual, inline errors when they occur (as opposed to a global error state). So when uploading an image fails, we should show the error directly in the uploading area. This inline element makes that easy to express.
The inline error badge can also show the error in question when hovering, so it's easy to dig into details when you hit an issue.
While styling this, I found I liked this as a lighter red background with darker text rather than a red border. I figured for consistency it would make sense to update `<ErrorNotice>` at the same time with a similar look. I like how the new error notice looks a lot more, it's less harsh.
Reviewed By: quark-zju
Differential Revision: D43411125
fbshipit-source-id: ae2c503f3e8c2f8e075cdf2fdd99d9a5bc6b19d2
Summary:
Actually add tests for all the image uploading features added in this stack.
We need to hardcode image uploading to be enabled for our tests, since it may be disabled in production / OSS builds.
These tests go over pasting/dragging/choosing files, making sure it sends the data to the server, and the placeholder&replacement system for the final uploaded links
Reviewed By: quark-zju
Differential Revision: D43408510
fbshipit-source-id: d581bb1df794e09bef15d944b52a01659f74bfb2
Summary:
To support adding tests for image uploading, we need to refine our mocks for VSCodeTextArea.
In production, we use VSCodeTextArea from the vscode-webview-ui-toolkit library. These are implemented using the microsoft fast framework, which creates custom HTML components, like `vscode-text-area`, which means they have a shadow DOM component.
For TextArea, this means the DOM structure is like this:
```
<vscode-text-area>
<label>...</label>
<textarea part="control"></textarea>
</vscode-text-area>
```
(with some other properties I left out).
So a ref of VSCodeTextArea will actually point to the outer `<vscode-text-area>`. But almost always we want to do stuff to the underlying textarea. (e.g. typing, focus). Some things work on either, but it's kind of inconsistent.
<tangent>
TBH, I don't like custom components when using react, it's just added complexity for minimal styling isolation benefit.
</tanget>
In our tests, we can't actually use the real VSCodeTextArea, due to some incompatibility with how it's bundled and how jest loads our code. Ideally we'd be able to fix this, but it doesn't seem to work just yet.
So we have a mock of our VSCodeTextArea component (all of our VSCode webview ui toolkit components, actually).
This mock could go through the efforst to make a custom component.... but jest doesn't support them well regardless.
Previously, we were being janky by just returning a <textarea> directly. That works until it doesn't. We started hitting situations where something would work in production but not in tests, because our tests didn't have the extra wrapper of the custom component.
Instead, we do something to mimic the extra structure. So in this diff I restruct our mock to wrap a text area:
```
<div>
<textarea part="control"></textarea>
</div>
```
This more closely matches the real structure of `<vscode-text-area>`. This means our tests can access the inner textarea while the rest of our code paths treat it as a real VSCodeTextArea wrapper.
In this diff I also clean up some references to `HTMLInputElements`, which are all actually `HTMLTextAreaElement`s.
Reviewed By: quark-zju
Differential Revision: D43408511
fbshipit-source-id: 1bd5c4638dca8cb87ce5ff6d145dbd12bcd103e9
Summary:
We had a bunch of small utils nested inside CommitInfoView.test.tsx. This test file is getting quite large, so I want to put new tests that relate to things inside the commit info view into a separate file. A nice first step is to extract these utils, so we can get the same ease of testing in other files as well.
inside this test file, it's implicit that we are always acting on the CommitInfoView, so we can destructure these utils at the top level so you don't need to qualify your uses with `CommitInfoTestUtils`. Other files may want to use the full name for clarity
Reviewed By: quark-zju
Differential Revision: D43408512
fbshipit-source-id: fbf2bb6348c809d001236aa4318e1f093acbf939
Summary: Hook up the server side handling to actually upload an image file.
Reviewed By: quark-zju
Differential Revision: D43378565
fbshipit-source-id: b9260bcb663a0426181d4b4c57ea2841c313deef
Summary:
Add a util that lets you extract one message from the stream of messages. You provide a message type and a predicate to check for. It returns a Promise that resolves when it sees a message that matches, then disposes its listener.
This is useful for "subscribing" to look for a message with a matching ID.
Reviewed By: quark-zju
Differential Revision: D43378566
fbshipit-source-id: 039641ad530fa6dc57b25f042a475fe962a91ea5
Summary:
To support uploading images to test plans, we need to send the file data over to the server.
We do this by sending ArrayBuffers to the server along the websocket.
Other ideas don't work as well:
- sending as text via base64 encoding is less efficient
- sending filenames so the server can read the file itself doesn't work because the server may be running on a different host than the client
ArrayBuffers are supported in websocket messages. However, they cannot be embedded into our JSON messages. Doing so would make the entire message binary and so JSON parsing would fail.
We also need to send context along with the binary data (what its for, metadata about the file, etc), so it doesn't fully suffice to just send the ArrayBuffer directly.
Instead, the idea is that we send two messages back-to-back. The client's API is to send a JSON message with a binary payload as a second argument. First, we send the JSON message that declares that it expects a binary message to follow it (plus any metadata, filename, how we expect to use it, etc). Then we send the binary ArrayBuffer.
We then reconstruct a message+payload on the server, and can put it into another `switch(message.type)` to handle it. This lets us support multiple kinds of binary messages in the future.
Note that websockets are a reliable transport, so we can guarantee messages will come in-order, so we never expect the message&payload to not come sequentially. If we ever saw that, it would be a programming error on our part and not a networking bug.
The use case at hand is uploading images to embed links. This diff includes the `uploadFile` type to kickstart the use case of sending binary messages. We'll need to include more info this message type in the near future: file name, file type, unique ID, etc.
Reviewed By: quark-zju
Differential Revision: D43369819
fbshipit-source-id: 59dcd03b2b891f54aa12f91b25fadfe292ce67d4
Summary:
Unfortunately, the `gh` cli does not currently support uploading images for PR descriptions. Doing so requires acting from the user's browser using their session cookie. It's technically possible for us to get users to use that cookie to make requests, but it's by no means an easy process.
This seems to be an often-requested feature of the gh cli, but they seem to not be interested in adding this feature:
https://github.com/cli/cli/issues/1895#issuecomment-718899617
for abuse and technical reasons.
Perhaps in the future we could support users configuring their own uploadable endpoints like a custom S3 bucket. Another variation would be to upload images to the repository via git push, then reference those file handles.
For now, we'll just disable image uploading features on non-internal builds.
Reviewed By: quark-zju
Differential Revision: D43369800
fbshipit-source-id: ef4cf083f6433bc795f2bf3488f476bed975b20c
Summary:
See previous diffs for context about image uploads.
Now that we support drag&drop and file uploads, its pretty easy to support pasting images as well. This is a relatively modern browser feature that lets you get the files attached to clipboard events. The API is quite simple, we just need to use `onPaste` and then hook into the existing file upload features from earlier in the stack.
Reviewed By: quark-zju
Differential Revision: D43332596
fbshipit-source-id: 9e5ee2e0dd8b47b694c20d67015995f047968ab8
Summary:
See Previous diffs for context about image uploading.
Now that we have a toolbar on the text field, we can add our upload progress to that toolbar, and even let you click on it to cancel the ongoing uploads.
We can accumulate this state from the ongoing uploads we know about. If any are uploading, we show a progress message.
If we click to cancel, we don't even truly cancel the uploads (because that might be fruitless), we just delete the placeholder texts and remove the in progress state.
Reviewed By: quark-zju
Differential Revision: D43332599
fbshipit-source-id: 8006ca752853167abac47aaea8a54a997c5b30f5
Summary:
See Previous diff for context about image uploading.
If a user doesn't know about drag&drop and pasting images, it's hard to learn about. By having an explicit *click to upload file* UX, it becomes discoverable. Also some users may struggle with drag&drop but can handle a click-to-upload.
(TODO: we should handle keyboard-only opening of the file picker for accessibility)
In this diff I add the notion of the EditorToolbar for a CommitInfoField. This is a button bar floating at the bottom of the text area that can include various buttons/editor status/messages. This is a common UI pattern for adding features to a text area.
Here we add the file upload button and later add the status of ongoing uploads and let you cancel ongoing uploads.
In the future we may add additional buttons to this toolbar, like filling from a template or whatever
The file picker itself is an icon button using a paperclip icon. That's the typical icon used for attaching files (email, facebook composer, etc). Surprisingly vscode codicons doesn't have a paperclip icon, so I had to make one myself. I styled it with width that roughly matches codicons. I might make a PR to codicons to add this icon.
The file picker is handled as an `<input type="file" />`. To customize rendering, we need to use a `<label>` & hide the input, so clicking on the label opens the input even though it's hidden.
Reviewed By: quark-zju
Differential Revision: D43332597
fbshipit-source-id: bfce7e9a866c2993e6280d0054c7751c8f366179
Summary:
Allow dragging an image file onto the text area in the commit info view. This will upload the image to the code-review-provider-specific image uploading server, and then insert the link to the file where your cursor was.
For Github, this will upload to github like if you dragged an image into a Pull Request editing area. And we have a separate image upload service internally.
This diff does not handle the file uploading itself. Instead, it just waits for 30s before returning the file name.
later in the stack, we'll add the rest of the infra to actually do the uploads of the files. The first few diffs in this stack will be setting up the mechanisms to get the files we want to upload and making the UX for it work.
This diff only adds support for drag&drop, but we'll also have a file picker and copy/paste support in the next diffs. Plus cancelling, progress, etc.
Drag & drop support is not too difficult, we mostly just need an `onDrop` callback on a wrapper div. We use onDragEnter/Leave to set a highlight class which lets us render a highlight over the entire droppable area, plus text prompting the drop.
More of the complexity in this diff is supporting the file upload state. Namely, how we show ongoing uploads and insert the resulting short link.
We want to insert the file where your cursor is. This comes from experience in the previous ISL implementation. It's not very nice to drop the link at the end of the text area.
If inserting at the cursor, since the upload takes time, we need to track that cursor location. Also from experience in the previous ISL, it's not as nice to drop the link whereever your cursor ends up once the file is ready.
A solution that gives a nice-feeling UX is to drop placeholder text where the cursor is, and replace the placeholder with the link once the upload succeeds. This keeps the link in the place you initially wanted.
The downside of this approach is that the user could possibly delete some of the text of the placeholder, preventing future replaces. This isn't a huge deal, but a better approach might be to use a contenteditable div so we can insert an HTML spinner that we can find&replace rather than literal text. Unfortunately contenteditable has its own quirks so I'd rather test out such a change later on.
We also need to be slightly careful about replacing the placeholder, so we preserve your selection. And lastly, we want to selectively insert spaces around the inserted text so that the link doesn't collide with other characters that make it an invalid link.
Reviewed By: quark-zju
Differential Revision: D43332600
fbshipit-source-id: e7453084f15e46caefb7a6a4c8d62e5f0419b85c
Summary: Another small refactor, separate CommitInfoField into its own file. We will be expanding the functionality of this text field quite a bit, so separating it makes it easier to follow, and the main commit info file can just render it as a text field without worrying about the details.
Reviewed By: quark-zju
Differential Revision: D43332601
fbshipit-source-id: a0c155690f87e95a770c3473d65051372371a73f
Summary:
Small refactor: move the atoms and some other utils into a separate file for the commit info view.
This will make later diffs this stack easier to work with. It was already getting to be a large file.
Reviewed By: quark-zju
Differential Revision: D43332598
fbshipit-source-id: 1d2e387dee9d49cd76ccba8a728bc3197a51c073
Summary:
Commit/amend fail if you pass in untracked files. If we pass `--addremove` these commands will succeed.
The CLI works by using tracked files as a way of specifying which files you want to include. But the UI is not really limited to such a system. We already have a checkmark system next to files you're going to commit/amend. That already represents the idea of "tracking" this file or not. It's purely extra overhead to think about if the files are actually tracked by `sl` or not.
In a subsequent diff we'll need to change how untracked files are rendered. No reason to show them as a question mark if it doesn't help you to make any decisions on your actions in the UI.
Reviewed By: quark-zju
Differential Revision: D43205299
fbshipit-source-id: 22cf28ec2ae55ec9ec599763597ddde818e5200a
Summary:
This is the last step in exposing aux data to EdenFS in trees. EdenFS still
doesn't do caching of these just yet, but that's coming in a subsequent diff.
Reviewed By: muirdm
Differential Revision: D43132762
fbshipit-source-id: 86488d0f071448e6e78d30e85e2963e001b1d67c
Summary:
This is unused, but we still allocate a Vec for it, remove it to save on the
allocation.
Reviewed By: genevievehelsel
Differential Revision: D43132767
fbshipit-source-id: 715e4b3e030887ccad464a2d881acf11802b1b45
Summary:
Now that aux data can be collected from tree fetches, let's move it closer to
EdenFS by having the backingstore collect them.
Reviewed By: muirdm
Differential Revision: D43132764
fbshipit-source-id: c74898a7886e927ed9ef81a28fead1ce6fca1df3
Summary:
On EdenFS, it is very common to get a Tree and then immediately after read its
aux data. This is so common that the lock on the aux data IndexedLog is
extremely contended, leading to most of the time being spent waiting on taking
it. To avoid this, we should expose the aux data already present in the EdenAPI
result and return it to EdenFS immediately.
For now, this merely introduce the API, future diffs will plug it deeper into
EdenFS.
Reviewed By: muirdm
Differential Revision: D43132763
fbshipit-source-id: 13d2158e6f2b6d52905ce684bc0f12096a95920c
Summary:
Hg specifies the "null" id to be empty context. Respect that.
This makes `debugexportrevlog` work in `debugdrawdag --no-files` repos.
Reviewed By: zzl0
Differential Revision: D43373719
fbshipit-source-id: 13bdcbc03480482eee812dc8c485debdeac43dc3
Summary:
This is now done in a lower layer in Rust. So it's no longer necessary in
Python.
Reviewed By: muirdm
Differential Revision: D43263811
fbshipit-source-id: db69a4da749a6b7a405ff388af0ed533721c3ec1
Summary: Simplify the logic since it now exists in Rust.
Reviewed By: muirdm
Differential Revision: D43263818
fbshipit-source-id: c0aef1960ae7c250ca10f86a64184df0fe4ba7ad
Summary: This makes it possible to use it in Python.
Reviewed By: muirdm
Differential Revision: D43263817
fbshipit-source-id: f92c651a349264d04f3fc92c35549010423a1ac2
Summary:
While `EdenApi` is used in production use-cases. There are a lot of cases in
tests (ex. `newclientrepo`) that the EdenApi should not be constructed. For
example, if the repo does not have a remote repo configured (path.default is
empty) then EdenApi should not be used.
In Python `repo.nullableedenapi` is used to avoid creating EdenApi in such
cases. However, the Rust code does not have such logic. This diff adds such
logic in Rust so it understands when not to create EdenApi.
Reviewed By: muirdm
Differential Revision: D43263816
fbshipit-source-id: acf79c1d9988f8760e34700874c92c6ae8d3f057
Summary: The `create_git_tree` method currently converts the given git tree hash into a Mononoke `BonsaiChangeset` and stores it in the data store. An implied pre-requisite in this case is the corresponding raw git tree object should already be present in the blobstore. However, this invariant might be violated. This diff modifies the `create_git_tree` method to check for the presence of the git object before converting it into BonsaiChangeset
Reviewed By: mitrandir77
Differential Revision: D43363613
fbshipit-source-id: 8a390bdc86e7a5e857d8a0d5e5cf4dcb34e50402
Summary: Instead of returning `anyhow::Error` with textual description, the `upload_git_object` method now supports typed-errors which will be useful when integrating with SCS. The tests have been modified accordingly.
Differential Revision: D43314540
fbshipit-source-id: 4cb3bc0f3ea37ce23a51f034bb79cf14c4889942
Summary: This diff implements the `create_git_tree` method which is responsible for creating Mononoke counterpart of the git tree object. Note that we do not create a new `bonsai_git` mapping since that is just for actual commits. Additionally, we do not log this new changeset to the scribe category since we don't want the commit tailers to be notified of this change. Except the `git_tree_hash` property, rest of the changeset properties get default values.
Differential Revision: D43306786
fbshipit-source-id: 850bf2dae8aaf525348d1a27a5ca73c51edb7af9
Summary: The rust Manifold client has additional options controlling the read behaviour which can be set through `ReadRequestOptions`. In particular, `use_parallel_downloads`, `total_timeout` and `num_retries` are quite handy when running backfiller jobs since they help in dealing with large file fetches. This diff includes these options as part of `EnvironmentArgs` so they can be provided for any Mononoke app / service.
Reviewed By: mitrandir77
Differential Revision: D43271892
fbshipit-source-id: 2ef78b272314aeb3dc9c9958b3c1a4e9dbffe2f4
Summary: Now that the fix for Manifold UAF is in place, the skiplist update command no longer needs to have a sleep after command completion.
Reviewed By: mitrandir77
Differential Revision: D43401604
fbshipit-source-id: c8f23de6622fd067df91be4f759269216fbd40ba
