Skip to content

Conversation

@maraf
Copy link
Member

@maraf maraf commented Aug 27, 2019

Resolves #27.

All searching is now in form of id: {searchTerm} tags:GitExtensions. This pre-filters packages marked as GitExtensions, than it loads a dependency graph (one query per package) and checks if dependency on GitExtensions.Extensibility exists.

Searching using nuget.org search syntax (https://docs.microsoft.com/en-us/nuget/consume-packages/finding-and-choosing-packages#search-syntax) is not ideal.

When search term is "g", it won't find "GitExtensions.SVN"; the user must type "git", or "extensions".
This behavior can be even seen on the web:

It seems it doesn't work well on MyGet.

  • In blog post from 2013 they said it should work, but from my observation it just ignores the "id part".
  • Using "id:{searchTerm}" requires to match whole packageId, nothing like "starts with" or "contains".

It doesn't work at all when source is a folder on disk.

  • Using "id:{searchTerm}" requires to match whole packageId, nothing like "starts with" or "contains".
  • Tags searching doesn't work.

Solution

  • On all feeds except local folder, load packages with tags filter, then apply id filter on loaded packages.
  • On local folder feed, load all packages from feed, then apply tags and id filters on loaded packages.
  • All local (in-app) filters are applied as invariant-culture ignore-case.

@maraf maraf self-assigned this Aug 27, 2019
@mast-eu mast-eu added this to the 1.0.0 milestone Aug 27, 2019
@mast-eu mast-eu self-requested a review August 27, 2019 22:01
@mast-eu
Copy link
Member

mast-eu commented Aug 28, 2019

Few questions / comments:

  1. How did you solve NuGet package is "Not compatible" #27? E.g. do you append tags: GitExtensions to all search terms? Do you first filter for tags and afterwards for dependency on GitExtensions.Extensibility?

  2. When search term is "g", it won't find "GitExtensions.SVN"; the user must type "git", or "extensions".

MS is testing a new search on nuget.org these days. Might be the reason for some strange results. https://devblogs.microsoft.com/nuget/new-and-improved-nuget-search/

  1. It doesn't work at all when source is a folder on disk.

What is the use case for this? Do you use it for debugging or is the PM internally doing this at some point?

  1. It seems it doesn't work well on MyGet.

Any idea why? If not, I'm inclined to take this behaviour as a normal bug - if it affects only not preconfigured feeds (i.e. any feed except nuget.org).

  1. A solution for the NuGet package is "Not compatible" #27.

Please write Closes #27 or Resolves #27. This way the issue will be automatically closed when the PR is merged. More info: https://help.github.com/en/articles/closing-issues-using-keywords#about-issue-references

  1. Is this PR a draft because you're still working on it?

@maraf
Copy link
Member Author

maraf commented Aug 28, 2019

Sorry for being so brief in PR description...

  1. All searching is now in form of id: {searchTerm} tags:GitExtensions. This pre-filters packages marked as GitExtensions, than it loads a dependency graph (one query per package) and checks if dependency on GitExtensions.Extensibility exists.

  2. Not sure, it seems like they split package id by pascal-case and dots and that it needs to match whole word (just observation).

  3. A folder on disk is a valid NuGet feed. I'm using it during debugging.

  4. Today I'm going to investigate, don't have any clues yet.

  5. Ook.

  6. It's a draft because it only works on nuget.org, today I'm going to give it a bit more time.

@mast-eu
Copy link
Member

mast-eu commented Aug 28, 2019

it seems like they split package id by pascal-case and dots and that it needs to match whole word (just observation).

I made the same observation. Searching for GitExtensions.SVN or id:GitExtensions.SVN or id:"GitExtensions.SVN" always gives results similar to Git OR Extensions OR SVN. Resulting in >10000 hits, although there is a package with the very same ID. Pretty useless for our case.
Anyhow, searching for id:{searchTerm} tags:GitExtensions seems to entirely avoid the flood of not matching packages 👍

@maraf
Copy link
Member Author

maraf commented Aug 28, 2019

@mast-eu Last thing I'm considering is skipping id search on feeds and filter packages in PM. It's quite dirty, but until we have hundreds of plugins, it will work. Thoughts?

@mast-eu
Copy link
Member

mast-eu commented Aug 28, 2019

Getting all packages with tags:GitExtensions from all configured feeds directly on startup? IMHO that's ok.

@RussKie
Copy link
Member

RussKie commented Aug 28, 2019 via email

@maraf
Copy link
Member Author

maraf commented Aug 29, 2019

I still need to fix two tests...

maraf added 5 commits August 29, 2019 21:26
- Split one big method into multiple smaller.
- Simplify conditions for continuing search on next page.
- Fix loading single package (method FindLatestVersionAsync) when local filtering applies.
@maraf
Copy link
Member Author

maraf commented Aug 29, 2019

I have removed NuGet.Client update, because the never version doesn't support reordering of feeds, which we currently use. We can do the update later...

@maraf maraf marked this pull request as ready for review August 29, 2019 19:30
@maraf maraf requested a review from RussKie August 29, 2019 19:30
public List<string> Tags { get; } = new List<string>();
public List<string> Description { get; } = new List<string>();
public List<string> Summary { get; } = new List<string>();
public List<string> Owner { get; } = new List<string>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these can be mutated outside the class and may lead to unexpected situations. Is this really required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but I don't get the point. There are supported filter parameters. As searching supports multiplicity for earch parameter, they are all defined as lists. The class is designed as mutable, as implementations of INuGetPackageFilter may add or remove values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maraf is right. In the current design these are all supposed to be public.

@RussKie
Copy link
Member

RussKie commented Aug 30, 2019 via email

- All package id and version are compared case-insensitive.
- Instead of String use string.
- Rename method of INuGetPackageFilter.
Copy link
Member

@mast-eu mast-eu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a lot of functional testing with this PR and everything is working as expected.

@mast-eu mast-eu merged commit 91c5b32 into gitextensions:master Sep 3, 2019
@mast-eu
Copy link
Member

mast-eu commented Sep 3, 2019

Thank you

@maraf maraf deleted the SearchByTags branch September 3, 2019 08:14
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.

NuGet package is "Not compatible"

3 participants