Summary: Implement the storage logic for chunking and storing raw `DeltaInstructions` into the blobstore. This will be used and tested when we implement the derivation logic for `GitDeltaManifest`
Differential Revision: D49144886
fbshipit-source-id: 6c775fa713f069793d31a1641dba5933d6e320a9
Summary:
The raw `GitDeltaInstructions` can be potentially quite large (half a GB in some cases). If these delta instructions are stored inline with the `GitDeltaManifestEntry` then the overall size of the `GitDeltaManifest` would become way too large even if its chunked. Instead of storing the actual instructions directly, we store the total number of chunks that would be created for storing the instructions. Note that we don't store the list of Chunks Ids anywhere. The IDs of the chunk can be determined based on the hashing scheme used. The ID is the hash of the actual_commit + actual_mpath + base_commit + base_mpath + index of the chunk. Except the index, all other information is available in `GitDeltaManifestEntry` and thus can be used to access all the chunks.
The follow up diff will contain the actual chunking logic for splitting up the blobs.
Differential Revision: D49098610
fbshipit-source-id: 5eeacb3913c45f07193def417a7471cdd836bb70
Summary:
These tests were categorized as flaky. Really, they weren't flaky but would fail consistently when run in parallel because of conflicting accesses to files.
Use tempfile to create a unique file per run and avoid parallel runs overriding each other's files.
Reviewed By: YousefSalama
Differential Revision: D49195323
fbshipit-source-id: 38b8ccd733325e1a61fc9ef43a53d00d77a22ecf
Summary:
I made the split range selector overflow-x scroll so it would behave on small screens, but this broke the actual selection process, which is much more important.
I think I'll maybe redo how this looks/works in general so we don't have to have these vscode dropdowns, they're a bit wonky.
Reviewed By: quark-zju
Differential Revision: D49215034
fbshipit-source-id: 0fd3855753106e468287c2a86d98cbe47953ad78
Summary: if you drag to select lines, we can highlight them in the same way as if you're hovering on a line. This gives the strong hint that you can click any arrow to move all the lines together.
Reviewed By: quark-zju
Differential Revision: D49214070
fbshipit-source-id: ab76823b2b4b9a13c65a391b02e74e42b615a126
Summary:
The "files" tab in the edit stack modal is powerful, but it's not quite polished like split is right now. I propose we hide the files tab for the next release or so, until we can bring some of the changes from split into this view as well.
I believe this is a reasonable thing to do because the functionality provided here is not strictly different to what is provided by split—it's just a sort of UX optimization for a certain workflow.
I think we can make this into something very cool and useful, but I think we should focus on the split UI at first. If we leave this in in a less polished state, we risk diluting the value of the split UI and making it feel confusing as a whole.
Reviewed By: quark-zju
Differential Revision: D49213505
fbshipit-source-id: 92ec023b0ea2fd655de0b9ecff3b99425937f633
Summary:
- only show expand if some are collapsed
- only show collapse if some are expanded
Reviewed By: quark-zju
Differential Revision: D49211368
fbshipit-source-id: 8e0bedefded58631a3537e8e0d915790ee42e238
Summary: I noticed that context menus where the originator of the event is in the right half of the scren are offset incorrectly
Reviewed By: quark-zju
Differential Revision: D49211369
fbshipit-source-id: 2fd8587b12a1887a11ce4effeb6f777784e6880b
Summary: Add state to collapse files in split. This is done in the parent and passed to children so we can add "collapse all" / "expand all" later
Reviewed By: quark-zju
Differential Revision: D49210250
fbshipit-source-id: 96e9e5bad2c586bc66e1bfab1467eec9c75a5dc1
Summary:
Moving the range selector to the bottom adds a little extra vertical height we can use to render more code
I also think the typical workflow won't involve changing this much. I know it made sense to have this as the "first" thing you see, but since the state is saved regardless, you can change the range whenever without consequence.
Reviewed By: quark-zju
Differential Revision: D49209436
fbshipit-source-id: 47a27609e7c896468303668b29ab0a16c2c4ffea
Summary: small text and font tweak
Reviewed By: quark-zju
Differential Revision: D49204233
fbshipit-source-id: e23d777d1135ad0df3094a837e5e7f9b42d631da
Summary:
Add synatx highlighting to the split view.
Our existing highlighting infra is generally well set up for this.
However perf is a bit of an issue here. Two issues I had to solve:
- there's a lot more files potentially visible here than we'd normally expect in the comparison view, so trying to highlight them all at once is expensive
- to solve this, I set up an intersection observer so only visible files are syntax highlighted. Browsers are smart and this doesn't affect scroll performance as far as I can tell, but it will syntax highlight only as you scroll to new files which is great.
- not completely ideal yet is that we have to highlight the entire file if any part is visible, so long files don't get much optimization from this. However, it's still much better.
- we were converting from file text content into an array of lines on each render, and when that's passed into the highlighting hook, it would cause highlighting to happen each render, which caused new tokenization, which caused new highlighting etc in a loop.
- the fix is to memoize the Array<string> representation we extract from the text. Then we only highlight when the actual text changes (e.g. from clicking an arrow to move a line)
With these fixes, performance is bearable, but I won't say it's particularly great. If you click an arrow on a large file, it may take 40-60ms to move that line.
Some further optimizations we could do:
- don't highlight context lines, only visible _hunks_. This would save some processing for large files with small changes
- we could highlight in a webworker, so highlighting is completely async and doens't affect the main thread
- we rerun both before & after highlights when you click a line. Technically, this may not be necessary, we could skip half the work when you only click a single line's arrow
I'll also note that we don't allow editing right now. I have'nt explored how this plays with the editing mode.
We can also enable this for the "files" tab, but I haven't done so yet.
Reviewed By: quark-zju
Differential Revision: D49203833
fbshipit-source-id: 90d9088da422d3ac50d42738f0797be410c43ab9
Summary: Line selection regressed when changing to using a <table>, we can easily re-add this though. We should also deselect after moving lines, otherwise your selection persists weirdly.
Reviewed By: quark-zju
Differential Revision: D49200526
fbshipit-source-id: 75159ec6688ac02a14f994fc26469e122cd8a030
Summary:
Add a button between columns in the split view to add new blank commits. This allows you to decide you want finer granularity splitting midway through, and not have to manually shift everything over to make room for a new commit.
One TODO:
The way this works, it's creating "real" commits as opposed to the fake commit added to the end of the split. So if you make an empty commit and run split, it will actually make an empty commit.
I guess we should filter out empty commits when we go to apply the stack edit...?
What's more, it seems debugexportstack doesn't like empty commits, it fails to load split after creating some empty commits in my stack
Reviewed By: quark-zju
Differential Revision: D49193835
fbshipit-source-id: 787c21e8f23ec309efb9fee76e8e6294eb941ca3
Summary:
Some columns were bigger or smaller based on their content. Let's just fix them each at 700px or so. This is a random number, but it looks about right to me. We want each file to be large enough to render "normal" content without wrapping too much, while also for sure showing more than one column at a time on most screen resolutions.
We can't really guarantee the screen resolution. We could make this column size a function of the screen width, like enfore between: 300px < 70vw < 700px, so it has a min size, but then is based on screen, and then caps out at 700px. I didn't go for this because it can be a bit confusing if you resize and everything shifts around.
Reviewed By: quark-zju
Differential Revision: D49194521
fbshipit-source-id: e40c31cfa09d2ca36330e9e9e9b1c481d9997d42
Summary:
Add buttons in file headers to move the entire file left/right. This is equivalent to pressing each line in the file's left/right buttons.
I originally tried to do this with `fileStack.remapRevs`, but I kept running into it violating an invariant on the number of revs. Perhaps there's an easy fix for that, it would probably be a simpler implementation of this.
I'd love to get eyes on the logic here. I think it's reasonable, and mostly copied from what happens when you click a single line's arrow. But it is doing an extra check for context lines, which the single line implementation doesn't need to do (because it only runs on known changed lines).
Reviewed By: quark-zju
Differential Revision: D49172191
fbshipit-source-id: e6fa3ccfed4337fac8776fd5a36df873d5911ad6
Summary:
Use the split out function from the last diff in the SplitFile component. This lets us delete all the duplicated code.
We need to bring forward a couple of changes from split into this, but they're relatively harmless, mostly just tracking which line indices are added/deleted/context.
Reviewed By: quark-zju
Differential Revision: D49168989
fbshipit-source-id: 4203b2396a97daf135a4a2983fbdd110fca4f665
Summary:
In D49156730, we copied <FileStackEditor> into <SplitFile>. This is a lot of duplication of somewhat involved logic.
Now that we iterated on <SplitFile> a bit, it's clear that most of the code can be reused directly. So let's split out the reusable code that computes all lines, buttons, and line numbers.
Then it's up to the caller to choose how to render those lines. Either as one big <pre>, or a <table>, or compute the ribbons for split view, etc.
There's a TON of parameters we need to pass here, which feels a little silly. I think it's better to pass a ton of params to fascillitate reuse than to not reuse at all.
note: another approach would be to just leave this in one giant component that we code all behaviors into. I personally was finding it getting quite large and difficult to parse through. Extracting the line rendering makes it ever so slightly more readable if you ask me, so this is kind of worth doing anyway.
Reviewed By: quark-zju
Differential Revision: D49168992
fbshipit-source-id: 3e2dd03f131dcd99db0bb955be89f4b017c7e5ff
Summary:
picks up a bunch of changes Wilfred made to the hack grammar upstream
https://github.com/slackhq/vscode-hack/compare/v2.13.0...v2.17.1
Reviewed By: bolinfest
Differential Revision: D49194977
fbshipit-source-id: fb21ccd8c6642e9a567344c29fce92957dfafa18
Summary:
adds an option to format the textmate grammar files. this makes it possible to diff them.
i made it an option instead of always pretty printing them because i'm not sure how they're used everywhere. if they get minified again later we should just always do this.
Reviewed By: bolinfest
Differential Revision: D49195406
fbshipit-source-id: 65945ad7341bd2aa012699d0a04c5d945e5667dc
Summary:
`FilenameClassifier.getDisplayNameForLanguageId` takes a language ID like `hack` and is supposed to turn it into a display name like `Hack`.
first, it has to turn the language ID into a textmate scope name like `source.hack`, so that it can look up the `source.hack` textmate config.
but it was using `findScopeNameForAlias` instead of `findScopeNameForLanguage` to map the language ID to scope name.
we already have the language ID, not an alias, so the indirection through aliases is unnecessary -- and in Hack's case it's broken[1].
so this diff makes `getDisplayNameForLanguageId` first treat its arg as a language id, and fall back to the alias.
this is the same approach that highlighting uses:
https://www.internalfb.com/code/fbsource/[f8f30ba6a489]/xplat/vscode/vscode-extensions/code-compose/src/chat/TextMateMarkdownRenderer.ts?lines=156-160
[1] for some reason, the `ini` config seems to define an alias for Hack Configuration (ok) but also `hack`. this causes `findScopeNameForAlias('hack')` to return `'ini'`! the textmate grammar should be fixed, but that's a different bug.
Reviewed By: bolinfest
Differential Revision: D49191522
fbshipit-source-id: b6c0844dbf8ad2ea40ea3411037482f40db4679a
Summary: This gets rid of a hacky workaround that was written when the Windows hg launcher used the old hacky batch script. Thanks to quark-zju's stack from D49163002, these workarounds are no longer necessary and all integration tests on Windows can use the actual buck-built hg.
Reviewed By: quark-zju
Differential Revision: D49171182
fbshipit-source-id: 63ea7cabd1b93782bf3aba351a37087b655a57ba
Summary:
The "originalNodes" is important for setting up the mutation / predecessor
information. For the split case, it should be set to the commit being split.
This allows `max(successors(hashBeforeSplit))` to return the split stack top,
not the stack bottom.
Reviewed By: evangrayk
Differential Revision: D49081992
fbshipit-source-id: 555f44dde3339fb63a48638ae82cee3759624702
Summary:
The `:libhg` is gone, to use modules in `edenscm`, one
needs to register the special module finder/loader.
Reviewed By: zzl0
Differential Revision: D49178770
fbshipit-source-id: de17e1252225ebf84cedea3e9371a87dd40a3b81
Summary:
I'd like to differentiate the rebase operations trigerred by `amend`
and `rebase`.
For example, if a user deleted new added changes from an early commit of a
stack and then amend it. We cannot tell if the user wanted to delete the change
or move the change to a later commit.
Reviewed By: muirdm
Differential Revision: D49157532
fbshipit-source-id: 1221e1a0191cbd77f3cdd2b1f871443f3202621c
Summary:
Previously, we made commits with multi-line descriptions non-editable in split. I think it's more useful to let you change the title, and just keep whatever the existing description is.
In the future, we should probably also show you the description somehow (on hover?).
Here, I make the editing operation on the commit title field only affect the title, which gets split out for the editor.
Making every commit title editable is also more consistent for the UI, so it's not like, "why is only this commit's title not editable?"
Reviewed By: muirdm
Differential Revision: D49161482
fbshipit-source-id: 8157577c7d704cd7e44ec6948075ff294f0f920f
Summary:
Showing the blank right commit makes it more clear that there's a place that your changes will move to, and makes the mental model of split more clear on first load, that there's a set of commits laid out horizontally which you can move lines of code throughout.
This way, we also show `1/2` for the first commit by default, hinting that there are more commits that you'll be making.
Reviewed By: muirdm
Differential Revision: D49159468
fbshipit-source-id: 41880b29416cef7ad15a89d0638d7c8e19feed1d
Summary: Since we're not planning on enabling the edit file content button in the super near term, let's just remove this extra code. Same for the side-by-side editor view. We can show these features for the "files" tab instead.
Reviewed By: muirdm
Differential Revision: D49158968
fbshipit-source-id: f27773a482547b30c40b1ee4ae17655b21466ca7
Summary: Making the arrow larger makes it a bit easier to see and makes it more obvious that the entire point of the split ui is these little arrows
Reviewed By: muirdm
Differential Revision: D49157187
fbshipit-source-id: 4af76076e4a487d098e5245a8c8307717e299a60
Summary: Also tweak the "beta" thing to give some better spacing and use a red color
Reviewed By: muirdm
Differential Revision: D49156729
fbshipit-source-id: c38508ebaf00fff113c945538a1fd9e841985b8c
Summary:
Before, we used `' '` to ensure alignment. But now that we have a table, it's sufficient to have an empty `<td />`. So make the gutters null when they don't have an arrow in them.
This also fixes user selection to not include the gutters, since the `' '` was selectable, but now it's not
Reviewed By: muirdm
Differential Revision: D49156738
fbshipit-source-id: 5a4aed9388f3bbbd8122323573c8c807d4e514df
Summary: Add some darkening to the gutters so that the boundary between file content and line numbers
Reviewed By: muirdm
Differential Revision: D49156743
fbshipit-source-id: 7015e76d9d1901678ddc949f36281e9687004b3d
Summary: Tiny fix: without this, split wouldn't let you scroll all the way to the bottom
Reviewed By: muirdm
Differential Revision: D49156734
fbshipit-source-id: 950577052aa68a62a7bc470763de1dc3c55d0a74
Summary:
I was considering changing the ~~~~ context expander to match the comparison view, but I found that I didn't like how it looked. So let's keep the ~~~~. One small thing we can do is make it more clear that clicking will expand, by showing some text on hover.
Admittedly, by using CSS for this, it's not translatable.
Reviewed By: muirdm
Differential Revision: D49156741
fbshipit-source-id: d7229b439b721f72054b432cf8102da9d85c5ef3
