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

Improve tags suggestions #877

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

psychob
Copy link

@psychob psychob commented Oct 10, 2024

Now tags support two methods of matching:

  • static - which loads all tags when requested and keeps them in cache until next reload
  • dynamic - which loads tags from backend

Other than that it also supports two methods of matching tags:

  • stars_with
  • contains

These setting are controlled with following environmental variables:

  • LD_TAG_FETCH_TYPE
  • LD_TAG_MATCH_TYPE
  • LD_TAG_MINIMUM_CHARACTER

Now tags support two methods of matching:
 * static - which loads all tags when requested and keeps them in cache until next reload
 * dynamic - which loads tags from backend

Other than that it also supports two methods of matching tags:
 * stars_with
 * contains
Copy link
Owner

Choose a reason for hiding this comment

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

Can you maybe elaborate what you are trying to accomplish or what specific problem you are trying to solve? In general I'm a bit hesitant with proceeding with this change, as it adds a lot of complexity for very little value. Especially with duplicating the search logic on the server.

I can see the benefit of a "contains" search mode and there is an issue where people have requested this before. But does it really make a big difference if all tags are prefetched from the server once in a while? And do users really need to be able to configure the minimum amount of characters to trigger a search?

Apart from that this change would need to be covered by tests somehow, and the options should be part of the user profile rather than per instance. But let's figure out what should be part of this PR first before making further changes on it.

@psychob
Copy link
Author

psychob commented Oct 15, 2024

So with this MR, i wanted to solve two problems - that i feel are somewhat interconnected.

First one: ability to search that tag contain some string instead of searching that tag starts with some string. I feel like i shouldn't be forced to remember where i put my string (at the start, in the end or in the middle), since software can keep track of it.

Second one, is to offload searching - at least as an option - to backend. My reasoning for it:

  • usually backend is more equipped to handle this type of task
  • by moving it to backend, frontend can do less work, and because of that be more efficient (speed and power and bytes transferred)

But right now I'm looking that my local instance is returning only 6 KiB of tags. So this change might not be needed.

@sissbruecker
Copy link
Owner

I think for most use-cases / regular usage the current approach is perfectly fine. I'm not up for maintaining two versions of the tag filtering to potentially save a few kB being transfered in most cases. If filtering stays on the client-side, then I don't see the need for adding a restriction on the number of characters as well.

I'm fine with adding an option for a contains-type search though. If you want to proceed please make it an option in the user profile that can be toggled through the UI / settings.

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.

2 participants