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

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

Open
neilb opened this issue Mar 31, 2021 · 19 comments
Open

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

neilb opened this issue Mar 31, 2021 · 19 comments
Assignees

Comments

@neilb
Copy link

neilb commented Mar 31, 2021

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
Copy link
Member

toddr commented Mar 31, 2021

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

@oalders
Copy link
Member

oalders commented Mar 31, 2021

I would add local/

@neilb
Copy link
Author

neilb commented Apr 1, 2021

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
Copy link
Member

atoomic commented Apr 27, 2023

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

@atoomic atoomic closed this as completed Apr 27, 2023
@neilb
Copy link
Author

neilb commented Apr 27, 2023

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
Copy link
Member

atoomic commented Apr 27, 2023

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.

@atoomic atoomic reopened this Apr 27, 2023
@toddr
Copy link
Member

toddr commented Apr 27, 2023

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

@atoomic
Copy link
Member

atoomic commented Apr 27, 2023

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
Copy link
Member

toddr commented Apr 27, 2023

@oalders asked for local/

@toddr
Copy link
Member

toddr commented Apr 27, 2023

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
Copy link
Member

atoomic commented Apr 27, 2023

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
Copy link
Member

toddr commented Apr 27, 2023

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

@toddr
Copy link
Member

toddr commented Apr 27, 2023

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

@metacpan metacpan deleted a comment from atoomic Apr 27, 2023
@atoomic
Copy link
Member

atoomic commented Apr 27, 2023

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

@atoomic atoomic self-assigned this Apr 28, 2023
@neilb
Copy link
Author

neilb commented Apr 28, 2023

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
Copy link
Member

atoomic commented Apr 28, 2023

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
Copy link
Author

neilb commented Apr 28, 2023

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
Copy link
Member

atoomic commented Apr 28, 2023

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
Copy link
Author

neilb commented Apr 28, 2023

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.

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

No branches or pull requests

4 participants