metacpan / metacpan-grep-front-end

Grep Front end code
GNU General Public License v3.0
13 stars 12 forks source link

(Add an option to) ignore inc/ directory #60

Open neilb opened 3 years ago

neilb commented 3 years ago

If you're searching for something that appears in the inc/ directory of distributions, then your search results can be swamped by those, and there doesn't seem to be a way to ask for files in inc/ to be ignored.

I can think of at least two options:

  1. Just ignore the inc/ directory
  2. Make consideration of the inc/ directory an option, with it disabled by default, but a checkbox so you can ask for it if you really want it
toddr commented 3 years ago

would you do inc or would you do other common paths too?

oalders commented 3 years ago

I would add local/

neilb commented 3 years ago

I'll have a think about other directories, but the thing with inc/ is that it contains modules that are almost certainly already on CPAN, so you can get a hit on one file over and over.

Given the discussion on #toolchain, I think have a checkbox for "ignore inc/ directory", and have it unchecked by default.

atoomic commented 1 year ago

this is a dupe of #61 (or the reverse) I m currently working on a prototype to ignore files or directories from the grep itself

neilb commented 1 year ago

This wasn't originally intended to be a duplicate. I think inc should just be ignored (that's this issue), and #61 is to give the end user the ability to ignore things. It could be an option like [x] ignore auto bundled stuff. Most of the time it's going to be the right thing to ignore it, and people should have to know about a feature to ignore things, and that they should specify inc.

So I guess you can see this as a specific instance of a general capability, but I don't think that's the right user experience / interface.

atoomic commented 1 year ago

I can see the checbox as being some extra sugar to avoid typing /inc in the ignore path list I'm currently adding. Reopening it, for now we will see once we have the ignore feature in place.

toddr commented 1 year ago

Seems like you could just filter it out of the git grep -l result when you do the actual grep?

atoomic commented 1 year ago
Screen Shot 2023-04-27 at 18 20 10 Screen Shot 2023-04-27 at 18 22 27

where for example you can use the following ignore: *.t, t/*, inc/* so inc/* becomes a special case of the ignore rules list note: if we give one checkbox to inc, what other should we consider? ppport.h, *.md, *.PL, ... ?

toddr commented 1 year ago

@oalders asked for local/

toddr commented 1 year ago

note: if we give one checkbox to inc, what other should we consider?

I think this is the critical question. we need an interface that allows people to say what they want but I'm not sure how to go about it. If we make it too open, do we allow them then to inject command line flags or bobby tables us?

atoomic commented 1 year ago

What I'm trying to say is that I think it makes more sense to leave a field where you can customize and ignore all rules you want rather than adding some pre-set rules, otherwise that list might become extremely long.

I can see adding some extra to the list:

inc/*
local/*
t/*
*.md
*.json
*.ya?ml
*.conf
cpanfile*
 LICENSE
MANIFEST
INSTALL
Changes
Makefile.PL
Build.PL
Copying
*.SKIP
*.ini
README
...

but if they are not enabled by default it does not make sense to list them.

Maybe we could consider a shortcut: ignore all cruft or something similar

toddr commented 1 year ago

one person's cruft is the thing another person is looking for :)

toddr commented 1 year ago

It might be simpler to have: ONLY search .pm and .xs/.c/.h files?

atoomic commented 1 year ago

We already have the option to filter files like *.pm or *.h the main value of the negative list would be to exclude ppport.h for example when performing a *.h

I think we should have both option available: positive and negative filter then we can gather some feedbacks and improve it but as you said someone request is going to different from another one...

even if we can probably see some useful and common patterns

neilb commented 1 year ago

I know you're going for a googlesque minimalism, but I think this would benefit from labels for each input, especially as once something's in the box, there's no indication for what they are. Something like the the following (though the placeholder text in the input elements would obviously change):

grep-mockup

Given we don't know what are all the ways people might want to constrain the search, and as Todd says, whether some people might want to include the things that I can't imagine someone ever wanting to include, then I think you should go for the simplest capability that lets people exclude the things that we know some people want to exclude, and then see how that works out.

Two thoughts related to the exclusion:

  1. Could pre-populate the exclusion list with a default set of things, so that by default people get least surprise / annoyance, but they can always clear out that if they do want to include one or more of those things in the search
  2. Leave it blank, but have a button to the right for "exclude boilerplate", and clicking on that will put the standard set of exclusions in the input.
atoomic commented 1 year ago

Two topics here:

A/ add some labels: good idea it clearly has some value, as previously I had to attach two screenshot for that exact reason note that I'm not sure how it would/could look like on the search page, probably a good idea to solve

B/ Pre-Set exclusion list I would lean toward first to add a checkbox to prefil the most common exclusion, until we realize we always need to use them.... and then consider to promote them as default

neilb commented 1 year ago

I would lean toward first to add a checkbox to prefil the most common exclusion, until we realize we always need to use them

I reckon pre-fill is the right approach. I would argue that the userbase for this tool is already skewed to the more CPAN-savvy end of the pool, and they'd rather do the most sensible thing by default. If you can poll the room there and come up with a list that everyone agrees with as a default set of things to exclude, then pre-fill with that list.

atoomic commented 1 year ago
Screen Shot 2023-04-28 at 14 01 44

I've added a checkbox which is going to populate a predefine list, this is wip and need some extra rules probably the current list stand as:

inc/*, local/*, t/*, *.md, *.json, *.ya?ml, *.conf, cpanfile*, LICENSE, MANIFEST, INSTALL, Changes, Makefile.PL, Build.PL, Copying, *.SKIP, *.ini, README

also notice the addition of title to the input text, which is not perfect but a mitigation before I can get a nice css/UI with the suggested labels

neilb commented 1 year ago

Not sure I'd include Makefile.PL in that list – pretty sure I've come across distributions where their only use of a module was in Makefile.PL. Less likely these days, with DZ, but there are plenty of dinosaurs still roaming the wild savannah of CPAN.