Skip to content
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

Fix usage of aliases for isValidRefName #1386

Merged
merged 1 commit into from Nov 3, 2020
Merged

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Nov 3, 2020

Currently on master searching chr5:1-100 does not work since chr5 is an alias

This fixes that.. This is a knock on from #1369

Note that having an integration test for this would be great. Have tried to do this here recently but didn't get it completed https://github.com/GMOD/jbrowse-components/compare/search_test

@cmdcolin cmdcolin merged commit a83ae36 into master Nov 3, 2020
@cmdcolin cmdcolin deleted the fix_aliases_searchbox branch November 3, 2020 19:40
@garrettjstevens
Copy link
Collaborator

Agreed, would good to get an integration test for this as it was definitely working at one point. This fixes isValidRefName, but I'd also like to fix allRefNames, which appears to be why isValidRefName stopped working. Something like this, maybe:

diff --git a/packages/core/assemblyManager/assembly.ts b/packages/core/assemblyManager/assembly.ts
--- a/packages/core/assemblyManager/assembly.ts
+++ b/packages/core/assemblyManager/assembly.ts
@@ -155,9 +155,11 @@ export default function assemblyFactory(assemblyConfigType: IAnyType) {
         if (!(this.refNames && self.refNameAliases)) {
           return undefined
         }
-        let aliases: string[] = []
-        self.refNameAliases.forEach(aliasList => {
-          aliases = aliases.concat(aliasList)
+        const aliases: string[] = []
+        self.refNameAliases.forEach((_canonicalName, alias) => {
+          if (!this.refNames?.includes(alias)) {
+            aliases.push(alias)
+          }
         })
         return [...this.refNames, ...aliases]
       },

@garrettjstevens
Copy link
Collaborator

Oh wait, didn't realize this was already merged. I'll open a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants