Summary: Right click a commit then choose "view changes" to open comparison view. This is actually really useful in my testing!
Reviewed By: quark-zju
Differential Revision: D45677198
fbshipit-source-id: 29e5fb2773e7f90c89febd2e71322f75d21da276
Summary:
The old ISL showed diff badges next to commit titles, inline, with wrapping in responsive widths. This was a little annoying because things wrapped individually, and so it was scan visually since each commit was probably differently placed than the others.
We currently have been showing commit's diff information always in a second line below the commit. This makes it very consistent to find information, at the cost of vertical space. This makes ISL generally less information dense, which is not great.
Especially if you have a wide screen, this feels unnecessary.
The new approach in this diff is a sort of hybrid approach. Small screen widths will have ALL commits wrap their diff badge information onto a second line. This allows the title to wrap along side the buttons, which means buttons are in a much more consistent place on the right side, without jumping down.
On larger screens (800px), we don't wrap the diff badges, and everything is in one line. For extremely long titles, we also then allow text wrapping, but the buttons stay in place instead of wrapping.
After some testing, I actually think this system is a lot more natural feeling. Things generally stay in the place you expect them to be, and all diffs are consistent. The wrapping point at 800px is something we could adjust, but it feels about right in my testing.
One question: we could make this configurable with the old behavior, so we allow you to choose if you want to "always wrap" or "fill horizontal space".
But I think this new way is much nicer in general, so I don't think we need a config for it.
Also note that instead of using total screen width (e.g. with a CSS media query or a simple document.body.clientWidth), we actually watch the main content area and take its width instead. This allows us to be responsive respecting the commit info sidebar opening/closing,.
Reviewed By: quark-zju
Differential Revision: D45671035
fbshipit-source-id: 423b96c28b7e3fcd7faa5301d15f45b34a1ce2be
Summary:
Outside of the HTTP endpoint specifically (which uses JSON), Scribe doesn't *actually* require the use of UTF-8 strings in its payloads.
To enable use cases that want to be able to offer non-UTF-8 messages into Scribe, the `ScribeClient` API has been changed to accept a more generic `&[u8]` instead of a strict `&str`.
Existing callsites that expected to only log UTF-8 strings now must enforce that at runtime or at their own API boundaries, which shouldn't be a problem in the scope of this codemod as that was (until now) enforced by the compiler.
Reviewed By: kuecks
Differential Revision: D45657053
fbshipit-source-id: bb08b02d3129e2cee895847c634fe9e809995f72
Summary:
The typeahead gives poor results when you only type one character, and those queries are much slower than two+ character queries.
Let's just limit our fetch to only happen for 2+ characters typed.
Some typeaheads even limit to 3+, but I think we can handle 2+.
Reviewed By: quark-zju
Differential Revision: D45665010
fbshipit-source-id: e4743550ee33352fd3a0cbe6f1df974513d1ec40
Summary: The suggestion typeahead was reimplementing the tooltip styles slightly, but didn't have the tooltip arrow. Might as well just use the tooltip styles directly instead by adding those classnames. It's not so complicated so I don't mind just re-adding the classnames.
Reviewed By: quark-zju
Differential Revision: D45664874
fbshipit-source-id: 5bfb1a98c0aa30dd3989911015493fe28fe273cf
Summary:
Small fix to make certain types of typeahead easier to read: if the "label" and "value" are the same, don't render the value as a `<Subtle>`.
Also, add a min-width so short tags in this format don't make the typeahead look really small
Reviewed By: quark-zju
Differential Revision: D45630154
fbshipit-source-id: 9bb24e9a678140cc574f9d46a7150c5d2c1f9fb6
Summary: An oversight in how we extract tokens means that you couldn't type spaces in fields with typeahead.
Reviewed By: quark-zju
Differential Revision: D45629080
fbshipit-source-id: 17be7b635af96b096fdf9c214118b731892feb76
Summary:
My initial rendering didn't handle long names well, it would shrink the image and also overflow outside the typeahead box.
You see this especially with weird reviewer groups for "bad" searches.
Reviewed By: quark-zju
Differential Revision: D45627730
fbshipit-source-id: f18c77005b61d7e51a158318f9ee7b9e71b91cba
Summary:
Simple logic that throws out data fetches if we've already shown a newer one. This greatly reduces jumpiness in my testing.
A common situation is that in typing "evan", each keystroke is firing a query, and the "e" query obviously returns a lot more results, and thus takes a lot longer. the "ev" or even "eva" queries may come back faster due to having fewer results to return.
But if we've already gotten "eva" results, there's no use showing "e" results, they're always worse.
Reviewed By: quark-zju
Differential Revision: D45627732
fbshipit-source-id: 36f3251c2ecc134eefac906fa52908bdea3256eb
Summary:
Simple way to ensure the typeahead field is not lingering and instead shows relevant info.
Note: onMouseDown comes before blur comes before onClick, so we need to use onMouseDown in order to register the click before the typeahead is hidden due to this new CSS.
Reviewed By: quark-zju
Differential Revision: D45627736
fbshipit-source-id: 4e97f295fc3fffae15b32253f6a95ad8dd12ac73
Summary: Easy, just need to delete the given token
Reviewed By: quark-zju
Differential Revision: D45627743
fbshipit-source-id: b8456458d8d1021babb1de51df5eb33d8ffda1e8
Summary: Tokens should be deletable. If you press delete it should prioritize deleting the entered text, but if there is none, then it can delete the previous token entirely
Reviewed By: quark-zju
Differential Revision: D45627741
fbshipit-source-id: f89fb86551fd757749dd71d77ca21901f4e91043
Summary:
Small thing, I noticed if result load and then you press more keys, it would show the loading spinner again briefly before the next results load.
It's much smoother if we only show the loading spinner before any results load
Reviewed By: quark-zju
Differential Revision: D45627734
fbshipit-source-id: 4cc5c049f158749dac81a3b12cab7cf94599ba75
Summary: You should be able to arrow key up/down through typeahead results, and later click or press enter to add them. This diff just adds the selection UI but not confirming.
Reviewed By: quark-zju
Differential Revision: D45627731
fbshipit-source-id: 1b06b362ed340fdbadf8f78b6841eda9ec08a53d
Summary: The old ISL only showed usernames, but it's a nice touch to show avatars as well. We get them "For free" in our query!
Reviewed By: quark-zju
Differential Revision: D45627733
fbshipit-source-id: 95958954f303879209ce511e139cfad09cbdf6d1
Summary: Making the API that each code review provider can use to define how to do their fields' autocomplete.
Reviewed By: quark-zju
Differential Revision: D45627738
fbshipit-source-id: 5e98a385164b614437b3eeb7fe5c12659ddfcde0
Summary: Hook up our typeahead fetch function to send a message to the server. This is where we'll be able to actually do our fetches.
Reviewed By: quark-zju
Differential Revision: D45627740
fbshipit-source-id: d5426b84fb420f06e72e6f6f583577f63b1242d5
Summary:
"autocomplete" means a single *inline* suggestion, like ios / github copilot / gmail
"typeahead" means a dropdown list with several results as you type, which is what we want
Reviewed By: quark-zju
Differential Revision: D45627729
fbshipit-source-id: 76ac37116db7ccf3d0e584861163ab9d9e6e25d5
Summary:
We want to show a typeahead/autocomplete while you're typing reviewers / subscribers / tasks/ etc. This will need to fetch data and show a list of values, then let you accept the results.
To make this nicer, we also want to show the values as tokens, instead of just text. These visual tokens distinguishes values from each other, and lets you click on the x to dismiss them more easily.
This is just the first steps in building this UX. Remaining in further commits, we need to actually fetch the right data depending on the type of field, handle the data, insert suggestions, let you delete suggestions, and deal with focus / other subtleties.
Reviewed By: quark-zju
Differential Revision: D45627742
fbshipit-source-id: d9133c04291bf6165bdf4b7efc6c22330ef68afe
Summary: Create a new field component for one-line fields which have typeahead support. This differs from the previous implementation, which was a multi-line thing
Reviewed By: quark-zju
Differential Revision: D45627739
fbshipit-source-id: 38294ac965ca2ec4e07b7aa23141e833b11c9951
Summary:
Re-implement the xrepo lookup as a namespace to map xrepo commits to local commits, and an autopull predicate to pull the translated commit.
There are a couple subtle concerns:
1. We don't want to trigger any xrepo work for normal cases such autopulling a normal commit. To achieve that, made it possible to defer the generation of an autopull attempt. This way, the xrepo autopull can cancel itself if the revision in question was successfully pulled by a higher priority autopull predicate.
2. We don't to trigger an xrepo query in `repo["deadbeef"]` (i.e. in commitctx.__init__). This could have surprising performance implications. To achieve this, we only do the xrepo translation during the autopull step. Later, the namespace lookup will only use the cached translation result.
I also moved the behavior into an extension to separate it better.
Reviewed By: quark-zju
Differential Revision: D45537964
fbshipit-source-id: 602db5218c618132672c708edabf366615646600
Summary: This test was flaky since a long time ago we made some changes to `drawdag`, which this diff fixes
Reviewed By: zzl0
Differential Revision: D45442022
fbshipit-source-id: 5a36a70f985d81c42f822f9933e0dc43d87cb591
Summary: I use these commands often but they don't have autocompletion.
Reviewed By: zzl0
Differential Revision: D45659357
fbshipit-source-id: 6f55324116cda45e159426f463974df175f9b25e
Summary:
strager pointed out the URL has `%2B` in it (so curl writes the file with the
`%2B` name) and the file name below uses `+`. Use `-o` with the desired file
names to avoid issues.
Reviewed By: muirdm
Differential Revision: D45620202
fbshipit-source-id: 27c9247fada2b702888d9f888c8e80c9e3e29a8d
Summary: There's nothing unix-specific about these tests, so enable them on Windows.
Reviewed By: genevievehelsel
Differential Revision: D45586168
fbshipit-source-id: 5acbcbe81c5c03b16e43fc561762dbafb47696b8
Summary:
If a user relies on an environment variable in their invocation of edenfsctl,
e.g. `--config-dir=$EDEN_DEV_STATE`, then if the user accidentally leaves the
variable unset they may inadvertently specify an empty string as the path to
their config dir.
When running `edenfsctl stop`, the effect of an empty config dir path is to
shut down your main eden daemon instead of your dev daemon, so this has proven
a bit of a footgun.
Reviewed By: xavierd
Differential Revision: D45591968
fbshipit-source-id: 6df6eb8c05d94dafcaee1d6976a52310454a42d4
Summary:
Add some test cases making sure changing cwd resets states properly. I can definitely imagine edge cases here so this will be a good place for future tests as well.
Note that I had to invert the nesting of <Tooltip> and <VSCodeRadio> to make the test mocks happier, it doesn't really affect much.
Reviewed By: quark-zju
Differential Revision: D45615005
fbshipit-source-id: e1398aa8e5918c42115f4562dfba946d095ee49f
Summary:
I noticed we had a million "possible EventEmitter memory leak detected. 11 listeners added" warnings in our tests, even though we disposed our event emitters correctly. That's because we legitimately had more than 10 subscribers to our cwd changing, which seems like way too low a limit.
I think it's better to just implement this "onChangeCwd" ourselves. This also may help us remove the eventemitter polyfill from the client side (it's actually a node thing, so webpack must be polyfilling it for us when we import it from events)
Reviewed By: quark-zju
Differential Revision: D45610780
fbshipit-source-id: e4fd994db4628975766b2faac2efc2b61009d004
Summary:
Some atoms in ISL should clear their state back to the default when the repository is changed. For example, the list of previous and queued operations. If your active repo is now a different repo, those operations aren't really comparable. So it's better for us to just hide them.
Here we introduce an atom that makes it easy to declaratively reset your atom state on repo change.
Reviewed By: quark-zju
Differential Revision: D45593018
fbshipit-source-id: ad3dd0eff875d54b1083d63ab8197a88e68a8f80
Summary:
After the last diff, we now support changing your active repo/cwd.
This requires the client to forget most of its state—in a new repo, none of this would apply. This could have been quite difficult, but we were already basically set up to support this via serverApi.onConnectOrReconnect. A slight superset of this is the new serverApi.onSetup, which handles connect/reconnect, but also handles changes to the cwd/repo.
All our atoms which depend on server state will then automatically re-send their init message, which will then get a response pretty quickly with new info.
This seems to work well in my testing, though it's possible that for there to be some subtle bugs here.
Need to dig in a big to look for anything that might not be getting invalidated. For example, the last run command in the progress thing at the bottom does not get cleared when changing repos. Kind of a tradeoff here, because we'll lose your history of commands since it's not persisted anywhere. That's probably ok though, better have it consistent.
Reviewed By: quark-zju
Differential Revision: D45592101
fbshipit-source-id: 609a1cea64034175514764f39429cf711a491939
Summary:
Now that we show a dropdown selector to change your repo, start handling the changeCwd message to actually update your repo.
This requires some slight restructuring. Instead of getting the repository reference in onClientConnection, we have ServerToClientApi know how to make repo references.
This is a slight inversion of control, but since we need repository reference creation to happen in response to messages from the client, it kind of makes sense to do it in ServerToClientAPI.
Reviewed By: quark-zju
Differential Revision: D45592197
fbshipit-source-id: cdc5f6ef63e608998b0e6483cf2b4454bce5065e
Summary:
Some platforms, like vscode, will allow you to change your cwd/repository among several options. In vscode, you may have multiple workspace folders. Each one should allow you to change your active directory.
We need to surface this list of options, based on vscode's mounted workspace folders, so you can click them to change the current repo.
This process will differ for each platform, although the UI can probably remain similar between them. So we hook this UI state up with platform-specific messages shared between client&server.
Reviewed By: quark-zju
Differential Revision: D45592198
fbshipit-source-id: 35a261370bc13f9feead3bbe9971f473c8701d13
Summary: Do not hard-fail when edenrc is corrupted
Reviewed By: xavierd
Differential Revision: D45550209
fbshipit-source-id: 88b5e7249a7da2ef4c6df69a4c65f08b7832d8d2
