First-contributions: Weird behaviour when using Atom to edit Contributors.md

[cryptographicfool]

🐞 Problem


I submitted a pull request by adding my name and GitHub profile link to the file Contributors.md, by editing the same in Atom editor.

However, if I check under "files changed" under pull request on GitHub, it shows that I changed multiple lines, shows many lines added and deleted (+23, -22). Why is this behavior happening if all I added was one line with the above mentioned details?

dart Goal

I see other users like me, who's pull request have not been merged, have the same behavior happening in their "files changed" section. I believe they were using Atom or something similar?

I could re submit using nano on terminal if required but the same should be mentioned in the otherwise brilliant tutorial.

[cryptographicfool]

Maybe the bot is ignoring many pull requests because of this? :(

[cryptographicfool]

deleted everything and stated from scratch. This time, I used nano on terminal instead of atom to edit the Contributors.md file and it got accepted and merged. So if someone is struggling they could try and use nano.

I still wonder why the weird behavior when using Atom.

[danmulvey]

I had the same problem using Visual Studio Code, deleted my branch and started over using sublime text to change the file and had no problem. I haven't had this problem with other files I've edited, not sure why it seems to be happening so much here.

[Moto42]

I think Atom has a problem with 'large' files.
Scare quotes, because large for Atom seems to start earlier than most editors.

[MatthewFallon]

Hey just wanted to add in because I have seen that this issue seems to have created issues for various pull requests.

@cryptographicfool I believe that this is the pull request you closed earlier that's in question:

17184

I noticed that the lines that get changed are actually the same ones for each person with the issue, and it might not actually be an issue for large files but with auto-formatting in this case.

If you take a look at the diff log for the contributor.md file in that pull request you'll notice that, other than the line you added, all the other lines were changed from having a space at the end of the line to not having one. Perhaps Atom does some auto-formatting to erase trailing whitespaces when you save the file, an action I would normally approve of right until it screws everyone that doesn't know to check for it :laughing:

Perhaps the README.md should also quickly teach people about running a git diff to check the changes they've commited, or maybe including a mention for Atom and any other editors that might do this. I run a pretty vanilla configuration of Intellij IDEA and it did not do this.

Or maybe even better, is there a way to have an auto comment like what happens for a successful merge happen when there are conflicts to bring up some of these common issues?

Whichever seems like the smarter option, kind of a newbie so idk which would help. although I know the maintainer of this seems to be busy, so that might pose a problem to adding documentation to it.

[cryptographicfool]

@MatthewFallon Indeed, it seems like Atom doing auto formatting to handle whitespaces.

The diff log shows that the space was removed and then added back. #17184 shows +24 - 22; 2 out of these are the two tries that I made to push via atom and the rest 22 are the trailing white spaces.

However, the fix was easy, re did the whole thing using sudo nano on the terminal.

[cryptographicfool]

I could open a pull request with edited README.md, prompting users to check git diff if their requests are not auto merged and a short guide to do so via terminal. But I am not sure if @Roshanjossey is around at the moment to merge it.

[MatthewFallon]

Sounds like a good idea, you can just make the pull request right now, and tag this issue to see if one of the maintainers will merge it, or I’ll do it myself later this evening :+1:

[MatthewFallon]

Added a quick pull request with demo output of the issue :+1:

[cryptographicfool]

Thanks @MatthewFallon

[Roshanjossey]

Hi @cryptographicfool, @MatthewFallon, Thank you for raising this issue and contributing to address it. I'm sorry that I was not around for a long time now. I'm getting back and trying to sort things out.

I can confirm that the bot was not merging changes because the editors mentioned auto-formatted a bunch of lines.

Right now, I'm going through the pull requests and asking users to try it again.

As for making changes in the tutorial, to address this, I have some points against it. Please hear me out and reply how you feel about it.

1. Auto merging is not a thing outside this repo.
   Maintainers of open source projects reviews all pull requests before merging. I'm just using a bot to make my life easy. Talking about auto merging in a tutorial that introduces new people to open source might not be a good thing to do. That's something specific to this repo.

2. Getting the pull request merged or having name in Contributors.md is not the main point.
   This is just something we have to make the tutorial hand-on and interesting. The main point is learning the workflow.

   1. Conflicts can be resolved by me or the user manually
      I can resolve conflicts from my side but I'm not a big fan of doing it myself.
      The best outcome here would be the user looking up what a merge conflict is and fixing it themselves.
   2. Keeping the tutorial short is a design decision that worked really well for us.
      There's a bunch of things that can be in the tutorial but is not in because we want to keep it short.

[Roshanjossey]

Closing this because of inactivity. If there are any concerns, comment below and we'll reopen and discuss about it.