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

Don't add file if plugin sets content to None #321

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

sth
Copy link
Contributor

@sth sth commented Feb 20, 2024

I was looking for a way to filter the files that get included in the new repository. Currently a plugin can modify the content, but not completely remove it.

A straight forward way for a plugin to show it doesn't want the file is to set file_data['data'] to None. In the main script this can also easily be handled: Only output the file if its data is not None.

The change is quite minimal and shouldn't interfere with any existing code.

@frej
Copy link
Owner

frej commented Feb 20, 2024

I like the idea, but I would like to see a test case where a file is removed using the plugin. The test case should also check that nothing bad happens when a plugin-deleted file is removed in a later mercurial commit.

@sth
Copy link
Contributor Author

sth commented Feb 22, 2024

Added a test that adds, modifies and removes a file in the hg repository and uses a plugin to remove that file when converting to git. The resulting git repository then doesn't contain any references to the "bad" files.

There are some empty commits in the resulting repository, because all the changes done in those commits were to "bad" files. This has no negative consequences despite maybe looking a bit odd at first sight. But it makes sense since all the changes in those commits were to files that got removed.

Copy link
Owner

@frej frej left a comment

Choose a reason for hiding this comment

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

LGTM apart from the copyright year...

t/file_data_filter-removefiles.t Outdated Show resolved Hide resolved
@frej
Copy link
Owner

frej commented Feb 23, 2024

Please squash the fix into the commit for the test case. Unless you protest, I can do that when I get time to start merging later today.

@sth
Copy link
Contributor Author

sth commented Feb 23, 2024

I squashed it.

@frej frej merged commit fb225c4 into frej:master Feb 23, 2024
4 checks passed
@frej
Copy link
Owner

frej commented Feb 23, 2024

Thank you for your contribution @sth.

@sth sth deleted the suppress-file-from-plugin branch February 23, 2024 17:27
frej added a commit that referenced this pull request Apr 7, 2024
When commit e63feee ("Don't add file if plugin sets content to
`None`") started calling the file_data_filter plugin method, in order
to make deletion of plugin-renamed files work, no-one thought about
updating the example plugins.

Thanks to @hetas discovering this.

Closes 328
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