Fuzzy finder: don't force URL to use git SHA revision#22133
Conversation
75cfed7 to
3ae24bd
Compare
3ae24bd to
5552f7c
Compare
Previously, the fuzzy finder always used URLs that pointed to the exact active git SHA (example: `@cfd1249`). This behavior caused two problems: - when using the fuzzy finder, your URL can't point to a branch name like `@main`. You have to manually edit the URL if you want it to point to a non-SHA URL. - if you activated the fuzzy finder in a non-SHA URL (like `@main`) then you always re-downloaded the filenames the next time you activated the fuzzy finder (because your URL changed to a SHA URL like `@cfd1249`). This commit updates the fuzzy finder to use the revision (`@main`) URL, if it's available. This change fixes both problems above.
5552f7c to
9164ed4
Compare
There was a problem hiding this comment.
Updated signature to allow an undefined revision field (when the revision is not defined in the URL, like https://sourcegraph.com/jdk/8/-/blob/java/util/Iterator.java#L113:18&tab=references )
This change fixes the URL rendering logic so that we no longer generate invalid URLs.
9164ed4 to
dcc01d8
Compare
| // Parse the URL here instead of accepting it as a React prop because the | ||
| // URL can change based on shortcuts like `y` that won't trigger a re-render | ||
| // in React. By parsing the URL here, we avoid the risk of rendering links to a revision that | ||
| // doesn't match the active revision in the browser's address bar. |
There was a problem hiding this comment.
Note: changes the the URL should always trigger a rerender, because the location prop should change (it's an immutable object, meaning it gets a new reference on every change). If it doesn't rerender, that may mean some component may have a bad componentShouldUpdate() implementation that ignores its updates, which would be a bug we'd fix ideally. Or maybe something else is going on.
Anyway I don't think you need to solve this, this is probably fine for now.
There was a problem hiding this comment.
Good to know. It was surprising when the component didn't re-render after triggering the y shortcut.
Previously, the fuzzy finder always used URLs that pointed to the exact
active git SHA (example:
@cfd12491d0f2675ac630e06a3a040b94f3946f70).This behavior caused two problems:
like
@main. You have to manually edit the URL if you want it topoint to a non-SHA URL.
(like
@main) then you always re-downloaded the filenames the nexttime you activated the fuzzy finder (because your URL changed to a SHA
URL like
@cfd12491d0f2675ac630e06a3a040b94f3946f70).This commit updates the fuzzy finder to use the revision (
@main)URL, if it's available. This change fixes both problems above.