Attempt auto-generated files being hidden by default #421

ivantsepp · 2022-05-23T02:44:35Z

This PR introduces a new feature to auto hide autogenerated files. It builds upon the recent commit of being able to collapse files (c1ffa46). The idea is to parse out autogenerated file patterns from .gitattributes (link) and automatically collapse matching files in the UI.

Note
This is a PR just to spark discussion! I'm not sure what the best approach is as there are many options. We can ask questions like:

automatically exclude these files in excluded_files.json (should speed up searches as these files aren't indexed)
add a button to hide and reveal autogenerated files
allow this list to be expressed in configuration

Thoughts on a feature like this? And what would the best approach be? Thanks!

salemhilal · 2022-05-25T14:01:55Z

I love this idea! Here are some initial thoughts:

I really like the idea of parsing out .gitattributes. linguist-generated annotations are already used to do essentially the same thing in Github, and mirroring that behavior here seems intuitive.
I am not sure if it makes sense to put this behavior behind a config param or not. I'm erring towards no, at least unless it becomes an issue for someone.
I think it might be a good idea to still index these files, but leave them collapsed (similar behavior to Github). Having files be excluded entirely feels to me like something you'd want to opt into separately. I don't usually want to see the output of generated files, but I do on occasion.

That's all my first pass on the details of implementation. Generally though, this feels like a feature I didn't realize I needed. I'm gonna read through the PR now.

salemhilal · 2022-05-25T14:08:05Z

searcher/searcher.go

+		ExcludeDotFiles:           repo.ExcludeDotFiles,
+		SpecialFiles:              wd.SpecialFiles(),
+		AutoGeneratedFilePatterns: wd.AutoGeneratedFilePatterns(vcsDir),
+	}


Just to clarify, you swapped the order of this block so that we don't create opt if there's an error pulling?

ah i swapped it because i needed wd.PullOrClone to be executed first so that when wd.AutoGeneratedFilePatterns(vcsDir) is called, it can find the .gitattributes file. wd.PullOrClone will populate the directory so without it, the first run of this flow will not see accurate results for autogenerated files

ahh gotcha, that makes sense.

salemhilal · 2022-05-25T14:11:04Z

vcs/git.go

+		if len(matches) == 2 {
+			pattern := strings.ReplaceAll(matches[1], "**", "*")
+			pattern = strings.ReplaceAll(pattern, "*", ".*")
+			filePatterns = append(filePatterns, pattern)


Nit: Could you add a test for this pattern-to-regex conversion?

ah i might change this logic (see my comment about regexes and using direct file paths instead). but either way, i will add a test for the code added in this file!

salemhilal · 2022-05-25T14:13:32Z

ui/assets/js/hound.js

@@ -693,7 +693,7 @@ var ContentFor = function(line, regexp) {

 var FileContentView = React.createClass({
  getInitialState: function() {
-    return { open: true };
+    return { open: !this.props.isAutoGenerated };


It could be nice to put an [AUTOGENERATED] badge next to the file path. That's not a blocker of course, but it would be a nice visual indication about why the file is collapsed.

Great idea, will do!

salemhilal · 2022-05-25T14:23:16Z

Ok! Two asks before I approve this:

I think the glob-to-regex logic is fussy enough that we'd want a test for it.
Can you add this feature to our README?

One other thought on the feature itself: it might be worth having an override for this feature in the config. For example, you might have a .gitattributes file, but you want a different set of files to be marked as autogenerated. Or, you might not be using git at all. In those cases, you could specify a list of autogenerated files in your repo config which would take precedence.

ivantsepp · 2022-05-26T13:51:22Z

Thanks for taking a look and glad you think this feature is worthy of being added! I agree we can follow Github's approach where we hide but still index/search on these autogenerated files.

I'll look into adding a badge, adding documentation into the README, and adding config override!

Actually for the glob-to-regex logic, I'm not sure it's the right approach anymore. I just realized that you could nest .gitattributes in your directories so parsing out only one file that's in the root isn't full coverage. What about running a shell command like git ls-files | xargs git check-attr linguist-generated. Then we would get specific path names and we wouldn't need to regex anymore (we could just do a simple string comparison). Thoughts on this approach? I like this too since we can use AutoGeneratedFiles instead of AutoGeneratedFilePatterns (shorter name).

(sorry for not responding earlier!)

salemhilal · 2022-05-26T14:20:20Z

@ivantsepp your response rate is still probably better than ours on this repo! That approach sounds more than fine to me. It looks like git check-attr accepts a -z flag to make its output be machine-readable, but in honesty I couldn't get it working as expected locally. I'm definitely in favor of that shell command approach regardless — the world is a better place with fewer regexes 😅

ivantsepp · 2022-05-27T03:10:01Z

Flushed out that approach and added a badge!

Added config override and added docs for that. Though I wasn't sure if I should add notes in the main README about this?

ivantsepp · 2022-05-27T06:08:38Z

vcs/git.go

+		}
+	}
+
+	if err := filesCmd.Wait(); err != nil {


Followed the pattern in the example in https://pkg.go.dev/os/exec#Cmd.StdoutPipe so hopefully this is right

ivantsepp · 2022-08-23T03:32:11Z

friendly bump! was hounding the other day and was reminded that this feature would be cool! Any thoughts on the revised PR?

salemhilal · 2022-08-24T13:03:10Z

@ivantsepp Thank you for the bump! I think these updates look good. I'll float this PR around with the rest of the folks to see if there are any objections, but otherwise I'll merge this and cut a release. Thank you again!

Attempt auto-generated files

7284255

salemhilal reviewed May 25, 2022

View reviewed changes

ivantsepp added 5 commits May 26, 2022 22:20

Add badge

ddfcece

refactor to use filenames and no regex

948203b

add config override

3844daa

add some docs

e31661d

add config test

0f82fb8

ivantsepp commented May 27, 2022

View reviewed changes

dschott68 approved these changes Aug 24, 2022

View reviewed changes

salemhilal merged commit d2f8655 into hound-search:main Aug 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attempt auto-generated files being hidden by default #421

Attempt auto-generated files being hidden by default #421

ivantsepp commented May 23, 2022

salemhilal commented May 25, 2022

salemhilal May 25, 2022

ivantsepp May 26, 2022

salemhilal May 26, 2022

salemhilal May 25, 2022

ivantsepp May 26, 2022

salemhilal May 25, 2022

ivantsepp May 26, 2022

salemhilal commented May 25, 2022

ivantsepp commented May 26, 2022

salemhilal commented May 26, 2022

ivantsepp commented May 27, 2022

ivantsepp May 27, 2022

ivantsepp commented Aug 23, 2022 •

edited

Loading

salemhilal commented Aug 24, 2022

Attempt auto-generated files being hidden by default #421

Attempt auto-generated files being hidden by default #421

Conversation

ivantsepp commented May 23, 2022

salemhilal commented May 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salemhilal commented May 25, 2022

ivantsepp commented May 26, 2022

salemhilal commented May 26, 2022

ivantsepp commented May 27, 2022

Choose a reason for hiding this comment

ivantsepp commented Aug 23, 2022 • edited Loading

salemhilal commented Aug 24, 2022

ivantsepp commented Aug 23, 2022 •

edited

Loading