mozilla / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
176 stars 278 forks source link

Readme update and small fixes #884

Closed patjouk closed 6 years ago

patjouk commented 7 years ago

I had a few issues trying to run brackets because I forgot to pass the --recurse-submodules flag while cloning the repo. I made it more obvious that you need to do it.

humphd commented 7 years ago

@patjouk Can I ask you about this change? I've never needed to do recurse-submodules when cloning, only recursive. I don't think we want to clone any submodules of submodules (e.g., I'm not aware that we use any). Maybe something has changed that I don't know about?

cadecairos commented 7 years ago

hmm @patjouk would you be able to provide some STRs for recreating the issue you ran into? I also haven't had to run the checkout with the --recurse-submodules command before. I typically forget the --recursive flag and have to run git submodules update --init --recursive

patjouk commented 7 years ago

When I clone the repo the first time, I forgot the --recursive. I got this error:

Tracing dependencies for: main
Error: ENOENT: no such file or directory, open '[..]/brackets/src/thirdparty/text/text.js'
In module tree:
    brackets
      language/LanguageManager
        file/FileUtils
          utils/Global

When I realized it was an issue about submodules, I did git submodules update --init --recursive, which solved my problem.

About the --recurse-submodule: I looked at the man page for git-clone but could find --recursive flag so I thought that maybe, it got replaced by the --recurse-submodule. Plus, since the docs say it's the equivalent of git submodule update --init --recursive, I thought it was a good idea to update the readme.

Now, maybe we don't need the emphasis of the --recursive at the beginning like I propose in this change, but it could be a good idea to at least add a troubleshooting section at the end with the error message and the git submodules update....

Pomax commented 7 years ago

Issueing --recursive during clone will work fine, so changing the instruction to tell users to use recurse-submodules changes instructions that work, without solving the problem that you identified =)

We can certainly add a section for people who forgot to issue --recursive during clone, instructing them to use git submodule update --init --recursive, but I doubt we'll be able to solve the real problem here: git should know that there are submodules, and it should report back to the user that a clone operation was performed without submodule initialization, which'll probably cause things to not work and they might still need to run submodule initialization manually.

I don't know if that's an issue that's been filed against git itself, but if it hasn't, might be worth filing.

So with all of that said, let's change this PR to keep the original instructions, but add a small troubleshooting section for people who forgot --recursive when they cloned, to teach them how to init the submodules manually after the fact.

gideonthomas commented 7 years ago

Interestingly, they removed the --recursive option from the docs in v2.13.0, however, this is not reflected in their changelog. This is either a bug in their docs (coz the flag still works in v2.14.1) or they are slowly deprecating that option and replacing it with --recurse-submodules. Definitely worth filing with them

patjouk commented 6 years ago

I've removed my previous changes and added a troubleshooting section at the bottom.

cadecairos commented 6 years ago

@gideonthomas any idea what's up with Appveyor?

gideonthomas commented 6 years ago

@cadecairos yeah it's because of a missing appveyor.yml file in this branch.

@patjouk if you rebase this branch onto master, it should fix it.

I selected the option to skip building branches that are missing the appveyor file so that we don't run into this again.

gideonthomas commented 6 years ago

Thanks @patjouk, this is awesome!