TODO: Add tests.

@utkarshgupta137 Thanks for the contribution. It would really help If you could add tests on this.

@sadick254 Done. I've also updated the corresponding PR in tabs package: atom/tabs#560.
There is a corresponding PR in tree-view as well (atom/tree-view#1367), but it Is taking me some time to get the tests running.

I'm not sure the 1 test failing for this PR is related to this PR at all.

Also sorry for being so awkward with the review, I'm really new to submitting reviews so the UI's a bit foreign at the moment.

@patricecoulon
I don't care if you're joking. This is an environment intended for cooperative attitude and support. Not blatant Islamophobia. I will report you should you keep that up. Just to reference Atom's code of conduct

in the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to make participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, nationality, personal appearance, race, religion, or sexual identity and orientation.

I personally have a zero tolerance policy so I'll be watching your profile for a while. If you thought someone's review or contribution needs improvement, please do everyone a favor and actually say something useful. For example, I don't like @aminya's review format personally, but they've contributed to some really cool projects and are an otherwise really cool person. What you just said was uncalled for.

@M3TIOR I've modified the existing test case to see if git-diff works when the repository is inside another folder. If the git-diff is working on a repository inside a project folder, then it will surely work for any other repository in any other folder structure. I do understand that I've converted the test condition from necessary to sufficient for this case. If you want me to, I can separate the two tests cases/suites.

I think that would be in the best interest of the project.

The following are some good principals to follow when writing unit tests IMO, but I deffer to the Atom maintainers since they have the final say anyhow:

• No known and verified reproducible project issue, should ever be without a unit test to prevent regression once it's fixed. Unless it's something too complicated to test for like a memory leak.
• No one should assume that a feature working in their environment will absolutely work in another until it's tested.
• And finally, unit tests should only test individual features. It's in the name.

Trying really hard to think of a way to word that last part that doesn't make me sound like a jerk. It's understandable how it could go over your head if English isn't your first language. But I can't really assume that it isn't your first language because most English speakers where I'm from don't really use it formally where I read it. And I have Aspergers so trying to parse informal English is a bit rough. What I'm trying to say, is that Islamophobic jerk who showed up here earlier, has made it impossible for me to feel as though I'm communicating in a way that's not insensitive. 😩

I don't want to assume you're ignorant, and I also don't want to assume you're not from America, because I don't know you at all. Not that being from a different country is bad. I'm just worried because most Americans I know would take offense to having their nationality or language fluency brought into question.

@M3TIOR I've duplicated the test suite with changes required for testing subfolder detection & have kept the original test suite as is.

Hey @utkarshgupta137. Apologies for not looking into this on time. I know this PR has been here for a long time, during that time Atom has seen a number of changes. Due to the changes, this PR has now become outdated. Is it possible for you to resolve the conflicts?

@sadick254 Done.

@utkarshgupta137 Thank you for resolving the conflicts. This looks good, however, there are some comments from @M3TIOR that have not been resolved. Can we get those resolved?

I rebased my spec for this commit. A brief note: I don''t know what happened between the last Atom version I worked on and the current development version though I could look through the commit history and trudge through to find a solution, but I really don't have the energy, please forgive my laziness at this juncture. For some reason all my async Jasmine specs are breaking. Specifically timing out repeatedly, and I can't get Jasmine to give me a longer timeout to see if there are any real bugs in my specs.

With that said, I hope this helps.

https://github.com/utkarshgupta137/atom/pull/1

@sadick254 It looks like the tests are failing at an unrelated stage: https://github.visualstudio.com/Atom/_build/results?buildId=116370&view=logs&j=e4115def-01ab-568c-a31a-bc15166ffabd&t=c4607463-cf32-4eef-9650-44237e8b1b61&l=542. Otherwise, it is good to go.

As much as I'd like to write off the errors within the module as a consequence of core modifications; why is it that only core tests are being run @sadick254 ? Shouldn't Jasmine be running the full sub-module tests? Maybe this comes down to a resource availability problem, or a test suite limitation, but if a submodule was edited in the last commit it should be tested as well as the core. Otherwise you may be introducing regressions.

I could potentially start working on a patch for the test suite to detect what the git repositories last modifications were, and automate the test suite to run their specs during merges or commits for those. If you think that's a good idea anyway.

@sadick254 It looks like the tests are failing at an unrelated stage: https://github.visualstudio.com/Atom/_build/results?buildId=116370&view=logs&j=e4115def-01ab-568c-a31a-bc15166ffabd&t=c4607463-cf32-4eef-9650-44237e8b1b61&l=542. Otherwise, it is good to go.

@utkarshgupta137 I re-ran the tests on the pipeline and it looks like the changes from this PR is definitely causing the failure, would you mind taking a look?

So, this may come down to something I did. Either when I rebased I did so over the wrong base; ie from a dev branch. Whatever the reason, I double-checked my original modifications on the release branches and they work. I'll work on rebasing this again with a stable branch like v1.59.0-dev or v1.58.0 some time in the near future.

This PR has been superseded by #23446