sindresorhus / hide-files-on-github

Chrome extension - Hide nonessential files from the GitHub file browser
https://chrome.google.com/webstore/detail/hide-files-on-github/lpnakhpaodhdkleejaehlapdhbgjbddp
MIT License
320 stars 36 forks source link

Add options page and ability to choose to only dim files and custom regex #23

Closed radiovisual closed 8 years ago

radiovisual commented 8 years ago

Adds an options page with the ability to optionally dim files or ignore specific files with a regex. Closes #2 and #3.

The old value of isHidden was replaced with toggleOn because the new dimming feature made the isHidden variable name confusing to read the code. So I opted for the convention of toggleOn.

The multiple css files were put in their own directory.

This pull request includes all the changes made in my previous PR. Hopefully that doesn't break some sort of github etiquette...

sindresorhus commented 8 years ago

Hopefully that doesn't break some sort of github etiquette...

It's usually better to keep PRs focused. The larger the PR is the harder it will be to get merged. No worries though. We can work with this.

sindresorhus commented 8 years ago

Can you fix the merge conflict?

radiovisual commented 8 years ago

Ok, no problem. I will get these changes pushed and fix the merge conflict.

radiovisual commented 8 years ago

Ok, so I have made all the requested changes. The only pending item on the list is fixing the merge conflict. I am new to merging conflicts with forks, so can you give me some quick advice? If you just give me the commands you would use, I can figure out the rest. I am sure I am overthinking this...but I don't want to make a mess of things either. :santa:

radiovisual commented 8 years ago

:/ I thought I was all confident that I took care of it, but now it seems I have somehow re-pushed all my old commits? Not what I wanted. What I did was make a copy of my options branch, then I rebased my options branch with my synced master (thinking that would help me resolve the conflict), then I merged the optionsmerge branch back into options. I have just learned the hard way that this isn't giving me the result I wanted. Any advice would be appreciated.

sindresorhus commented 8 years ago

The easiest at this point would be to just copy paste the files into a temporary directory, create a new branch based on master, copy in the files in the temp directory, commit, and then force push to the remote branch of this PR. (No need to open a new PR btw). That way you get one clean commit based on master with your changes and you don't have to deal with a complicated merge resolving.

radiovisual commented 8 years ago

Ok, that sounds like a good plan. Thanks a lot.

radiovisual commented 8 years ago

Hm. I followed your advice to the letter, even making sure that my master branch was synced with upstream/master before making the new branch, and when I force push with git push -f origin options I just get the message:

Everything up-to-date

I even tried it twice, getting the same result each time. So now I am truly stumped.

radiovisual commented 8 years ago

However, now when I check the status of my two branches options and fmerge (the branch I created to follow your advice), they both say they can be automatically merged. The only place where the merge conflict warning shows up is here in this PR thread.

sindresorhus commented 8 years ago

If you did correctly, this branch https://github.com/radiovisual/hide-files-on-github/commits/options should have been the same as https://github.com/sindresorhus/hide-files-on-github/commits/master, but with one extra commit. Make sure your local branch is exactly like this master, not yours, then do one commit with the changes, and then force push to your remote options branch. fmerge is also incorrect and still contains old commits so it would not merge cleanly either.

radiovisual commented 8 years ago

Ok, thanks. I will try it again. On a positive note, this branch of the extension has been thoroughly tested "in the wild" because I have been using it on a daily basis since I pushed my last commit. So at least there's that. :)

radiovisual commented 8 years ago

phew. What a mess that was. Sorry for ugly commit history. I should have been more mindful about polluting the commit history, but I had to wrestle to get this merge conflict sorted.

I did uncover one problem when the options page initially loads, for some reason it is stripping out the first escaping backslash \ from ^\.|^license|^appveyor\.yml on initial load of the extension. So I need to address that now. The extension works fine other than that, as far as my tests go.

sindresorhus commented 8 years ago

You changed the file permission of a lot of files 100644 → 100755 in https://github.com/radiovisual/hide-files-on-github/commit/1add489388e8b4277e9106822d8465f9fa81d06a

radiovisual commented 8 years ago

:flushed: :sweat_smile:

sindresorhus commented 8 years ago

Awesome! Superb work on this @radiovisual. It turned out really good :)

sindresorhus commented 8 years ago

Added you to the repo. Feel free to commit stuff directly.

radiovisual commented 8 years ago

Awesome, thanks so much @sindresorhus! I am happy to continue helping when help is needed. I use this extension everyday. :smile:

sindresorhus commented 8 years ago

I noticed one bug. When a repo loads, the button "Show dotfiles" doesn't work. It works if you navigate to a file and then back.

radiovisual commented 8 years ago

Ok, let me take a look.

radiovisual commented 8 years ago

Hm, I am unable to recreate this problem. It works for me every time I load a repo. I will install the extension on another machine to test.

radiovisual commented 8 years ago

Everything I have tried, I am not able to recreate the problem. I have uninstalled and reinstalled, then I installed the version that is currently on the repo, and I installed the version that is sitting in my local options branch, everything works as it should on this end. Is there anything special about your environment that could be interfering? Have you disabled or deleted the old version of the extension? I recall that the old version (from my previous PR) had an issue with the Toggle button, could it be that?

sindresorhus commented 8 years ago

Weird. I tried restarting Chrome and it suddenly worked now. Sorry for the noise. Weird though as restarting or reinstalling the extension did not work. Sorry for the noise.

radiovisual commented 8 years ago

Hm, that is weird. It would be nice to know what could have caused it if it's something on chrome's end. AFAIK, there shouldn't be any extension caching that can get in the way, once an extension is deleted, it's gone. Weird. I will keep this behavior in mind for future reference, in case it pops up again and we need to take a deeper look at chrome.

sindresorhus commented 8 years ago

Alright. Published a new version now.

radiovisual commented 8 years ago

:+1: