cfx init should ignore hidden folders

RESOLVED FIXED

Status

Add-on SDK
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: KWierso, Assigned: warner)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
A user on IRC was getting ready to set up a package in the local SDK. 

He ran bin\activate, and made a folder for his package, then changed to that directory and ran "cfx init".

It then complained about it not being an empty directory.

After creating the folder, the user had added it to a Subversion repository, so it had a ".svn" folder in it prior to running "cfx init".

If ignoring hidden folders is too much to do, maybe just whitelist folders like ".svn", ".hg" (and whatever git uses)?
(Assignee)

Comment 1

7 years ago
sounds like a good idea to me. Probably just ignore /^\./ -matching items. The change will go into python-lib/cuddlefish/__init__.py in the "initializer()" function, something like:

 existing = [fn for fn in os.listdir(path) if not fn.startswith(".")]
 if existing:
  print >>err, 'This command must be run in an empty directory.'
  return 1
(Assignee)

Comment 2

7 years ago
Created attachment 514209 [details] [diff] [review]
patch, with tests

this patch should fix the problem, however let's keep bug 613604 (cfx init newdir) in mind as the preferred way to avoid this sort of thing
Assignee: nobody → warner-bugzilla
Tested the patch. Works for me. Thanks Brian!
Duplicate of this bug: 643673
Someone else fixed a special case of this with pull request 132 <https://github.com/mozilla/addon-sdk/pull/132>.

Brian: any reason not to get review on your patch at this point?
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Comment 6

7 years ago
nope, if it worked for Mihai, let's r? it.
(Assignee)

Comment 7

7 years ago
Comment on attachment 514209 [details] [diff] [review]
patch, with tests

Irakli, want to take a look at this?
Attachment #514209 - Flags: review?(rFobic)
Comment on attachment 514209 [details] [diff] [review]
patch, with tests

Looks good ;)
Attachment #514209 - Flags: review+
Irakli: can you also check this in?
Keywords: checkin-needed
Landed
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(In reply to comment #10)
> Landed

Can you provide a link to the changeset?
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.