dotnet-suggest: Don't duplicate registration entries, don't return false matches, and be more careful about casing
Los mantenedores suelen responder en 1 día
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Aptitud para principiantes
- 45/100
Línea de trabajo
Comienza con el registro de dotnet-suggest y FileSuggestionRegistration.FindRegistration(), especialmente con la ruta de registro y el bucle de búsqueda por prefijo. Verifica el comportamiento actual para entradas duplicadas, coincidencias primeras frente a últimas, prefijos falsos y uso de mayúsculas y minúsculas entre plataformas. Se considera terminado cuando el registro evita duplicados, la búsqueda devuelve únicamente la entrada prevista sin continuar innecesariamente y el uso de mayúsculas y minúsculas sigue el comportamiento de la plataforma seleccionada.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
dotnet-suggest is pretty simple in its handling of registrations.
It's just a utf-8 text file with one executable path per line that gets scanned line by line from start to end when searching, returning the last "match" found (which is also just a StartsWith match, and thus can return false positives), and which blindly appends to the file on registration, so long as the provided path exists. It also performs its search explicitly as StringComparison.OrdinalIgnoreCase.
These have a few undesirable consequences:
- Entries are duplicated if the same value is given to
dotnet-suggest register --command-path <command-path> - The search for a match continues even after a match is found.
- False matches can be returned, such as if there is a registration for
/path/to/fileand/path/to/fileThatIsNotTheOneYouWant, which will return the last of those two entries, even if it is/path/to/fileThatIsNotTheOneYouWantbut it was searching for/path/to/file. - The
StringComparison.OrdinalIgnoreCasestring comparison can result in invalid completions on case-sensitive file systems, which could lead to failure to launch the target to retrieve the completions for it, if the command line text does not match the case of the actual executable but does match the case of the entry in the list. Further, if two files differ in name only by case (which is also legal on Windows, BTW), or if you have a directory structure that looks like the following example, false matches can also occur:
/a/b <-- This is a file named b in directory /a
/a/B/c <-- This is a file named c in directory /a/B/
Searching for /a/b will, with the current code, return the second entry as the match, due to both the case-insensitive search and the last-one-wins match strategy.
(1) can be avoided at registration time by just making sure the entry doesn't already exist. It doesn't really matter if the registration operation is made more expensive by doing so, because registration isn't being done frequently, and isn't being done in a context where a brief delay to perform that scan is objectionable.
(2) and (3) can be resolved by returning as soon as a match is found.
It seems to me that it is most important for it to be optimized around the search case, since that is what gets invoked more frequently and in a context wherein the user expects as little delay as possible.
(4) just needs to either have an OSPlatform guard to perform case-sensitive searches on not-windows and case-insensitive searches on Windows, or else simply always perform StringComparison.Ordinal searches on all platforms.[^SensitiveSubject]
There are plenty of other pretty low-complexity ways to make it even smarter than the above simplistic suggestions, such as maintaining the list in sorted order when registering and then performing searches by binary search, and/or by using fixed-width lines to make seeking within the file possible without having to read the whole thing into memory (which would make doing a binary search even more efficient for example). But even just de-duplicating it and returning the first prefix match rather than scanning the entire file and then returning the last prefix match indiscriminately would be improvements over the current implementation, and actually result in fewer lines of code than are currently there, since that temporary string variable named completionTarget in FileSuggestionRegistration.FindRegistration() would go away entirely, along with the extra check for that variable being null that is after the loop.
[^SensitiveSubject]: Windows is case-aware and file naming is case sensitive in Windows, so this would be the most technically correct (the best kind), though of course there is the obvious argument that, since everything else about Windows is case-lenient, enforcing case-sensitivity could be confusing, due to user expectations.
- Lenguaje dominante
- C#
- Estrellas
- 3.7k
- Forks
- 436
- Merge medio
- 6 h 27 min
- PR fusionados (30 d)
- 2
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de dotnet/command-line-api
-
German localization is incompletePosiblemente ocupada @b-v-d-e-v la tomó hace 21 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 65/100
dotnet/command-line-api#2852 ·
Los mantenedores suelen responder en 1 día
-
Incomplete French (fr) translation: RequiredOptionWasNotProvided not translatedPosiblemente ocupada @JPBlanc la tomó hace 119 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 65/100
dotnet/command-line-api#2822 · 1 comentario ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 62/100
dotnet/command-line-api#2792 · 2 comentarios · 17 reacciones ·
Los mantenedores suelen responder en 1 día
-
`Argument.Arity` modify internal state on first get access.Posiblemente ocupada @zjdangge la tomó hace 5 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 62/100
dotnet/command-line-api#2704 ·
Los mantenedores suelen responder en 1 día
-
GetCompletions should check exit code of invoked applicationPosiblemente ocupada @baradgur la tomó hace 1290 días. AbiertoArea-Completions bug help wanted
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
dotnet/command-line-api#2137 · 1 comentario · 3 reacciones ·
Los mantenedores suelen responder en 1 día
Todos los issues de dotnet/command-line-api
Issues similares
-
clawsweeper:fix-shape-clear clawsweeper:needs-maintainer-review clawsweeper:no-new-fix-pr clawsweeper:not-repro-on-main impact:ux-release-blocker issue-rating: 🦪 silver shellfish P0
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
openclaw/openclaw-windows-node#1843 · 1 comentario · 1 reacción ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 66/100
-
kind:ux size:S type:bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
elsa-workflows/elsa-foundation#3256 ·
Los mantenedores suelen responder en 1 día
-
[Bug]: Revit selector reads process ids 2000-2099 as years (flaky AgentClientTests.AYearOrAProcessIdSelectsTheHost)Posiblemente ocupada @Wurschdhaud la tomó hoy. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 83/100
pyrevitlabs/pyRevit#3817 ·
Los mantenedores suelen responder en 1 día
-
bug effort:S P2
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
nightscout/nocturne#2449 ·
Los mantenedores suelen responder en 1 día