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

Update githelp to advise when sl clean --dirs is necessary #575

Closed
wants to merge 3 commits into from
Closed

Update githelp to advise when sl clean --dirs is necessary #575

wants to merge 3 commits into from

Conversation

steveluscher
Copy link
Contributor

@steveluscher steveluscher commented Mar 21, 2023

Whenever there's a path or the -d flag, git clean implies that cleaning should be done recursively, into directories, removing empty ones as it goes.

The equivalent in Sapling is sl clean --dirs.

Closes #564.

Summary:

* `clean` with paths implies `--dirs`
* `clean` with `-d` implies `--dirs`
* `clean` with `-x` implies `--ignored`

Test Plan:

CI run.
Summary: See tests in previous commit.

Test Plan: CI run.
@steveluscher
Copy link
Contributor Author

I haven't actually run the tests yet, because I was too lazy to figure out how. Kind of hoping you'll just mash the ‘run workflows’ button on this PR. :)

@vegerot
Copy link
Contributor

vegerot commented Mar 21, 2023

It's really easy to run tests (but undocumented >.>)

python run-tests.py test-fb-ext-githelp.t

@steveluscher
Copy link
Contributor Author

~/src/sapling/eden/scm/tests$ python3 run-tests.py test-fb-ext-githelp.t
usage: run-tests.py [options] [tests]
run-tests.py: error: --local specified, but sl or hg not found in '~/src/sapling/eden/scm' or not executable

I couldn't build, and didn't want to solve the 2^53 errors that the build process spit off. If you can mash the ‘run workflows’ button that would be awesome.

@steveluscher
Copy link
Contributor Author

Ah, solved the last of my build problems with #577.

@steveluscher
Copy link
Contributor Author

OK, I got the tests running but I don't really know what I'm supposed to do here. The tests presume Mercurial, and the runs replace everything with sl so they fail. Halp?

   $ hg githelp -- git checkout --force
-  hg revert --all
+  sl revert --all
 
 githelp for grep with pattern and path
   $ hg githelp -- grep shrubbery flib/intern/
-  hg grep shrubbery flib/intern/
+  sl grep shrubbery flib/intern/
 
 githelp for reset, checking ~ in git becomes ~1 in mercurial
   $ hg githelp -- reset HEAD~
-  Mercurial has no strict equivalent to `git reset`.
-  If you want to remove a commit, use `hg hide -r HASH`.
-  If you want to move a bookmark, use `hg book -r HASH NAME`.
-  If you want to undo a commit, use `hg uncommit.
-  If you want to undo an amend, use `hg unamend.
+  Sapling has no strict equivalent to `git reset`.
+  If you want to remove a commit, use `sl hide -r HASH`.
+  If you want to move a bookmark, use `sl book -r HASH NAME`.
+  If you want to undo a commit, use `sl uncommit.
+  If you want to undo an amend, use `sl unamend.

@zzl0
Copy link
Contributor

zzl0 commented Mar 23, 2023

Thanks for your PR, taking a look

Copy link
Contributor

@zzl0 zzl0 left a comment

Choose a reason for hiding this comment

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

Just a small comment, I will import it and fix it, thanks for your contribution.

@@ -384,8 +384,10 @@ def clean(ui, repo, *args, **kwargs) -> None:
args, opts = parseoptions(ui, cmdoptions, args)

cmd = Command("clean")
if opts.get("d") or len(args) > 0:
cmd["--dirs"] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

For Sapling, --dirs only delete empty directories, so I think need to set both --dirs and --files option.

@facebook-github-bot
Copy link
Contributor

@zzl0 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@steveluscher
Copy link
Contributor Author

Thanks! It's been a bit over a year since I've submitted a diff to Phabricator.

@zzl0 zzl0 added the fix ready A fix was created. It's pending review or push. label Mar 25, 2023
@facebook-github-bot
Copy link
Contributor

@zzl0 merged this pull request in fad2239.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed fix ready A fix was created. It's pending review or push. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sl clean --all ., doesn't.
4 participants