Remove case-sensitivity from language input

[ExcaliburZero]

Recently I was trying to use forkability to analyze a project that was written in Python, such that I would recieve the Python specific suggestions.

When I was entering the project's information on the web app, I inputted the language for the project as "python" with the first letter lowercase and it gave the Python requirements (ex. setup.py file). However, when I entered the language as "Python" it did not give the Python requirements.

Thus it seems that the language input box is case-sensitive, such that it will not recognize programming languages if they are written with the first letter capitalized, rather than being in all lowercase.

Could this be changed so that the programming languages could be recognized despite their capitalization?

[M-Zuber]

It probably can be done. Would you be interested in doing it? I am guessing if you lowercase things here it should be enough

[ExcaliburZero]

@M-Zuber Okay, I've cloned the repository and made changed the line you noted so that the language string should be converted to be all lowercase before it is checked if that language exists yet.

However, when I try running it with make serve, the application works, but it still seems to be case sensitive for the languages.

Here is the commit I tried: https://github.com/ExcaliburZero/forkability/commit/7737263bac607855762cc9b8e7ae932499caf6aa

[M-Zuber]

I left you a comment @ that commit. You might want to consider adding some tests :wink:

[ExcaliburZero]

Okay, I have changed the lowercased language name to be a local variable and replaced the instances of options.languages[i] with the new variable.

https://github.com/ExcaliburZero/forkability/commit/af027436ed6857ed05cfdf019eedfaa49c889a40

I don't know much about the unit testing system that this project uses, but I copied and modified one of the existing tests so that it should test upercased language names. Let me know if there is a better way this test should be implemented.

https://github.com/ExcaliburZero/forkability/commit/3d5c42024142119026125f86650b2a3b30ca0f44

I ran the test with having removed the change to lowercase and it fails, and by adding the change to lowercase back in, it seems to work. So the test seems at least functional.

However, when I run make serve and test the web app on localhost:3000 the language input is still case sensitive, in that when using a capitalized name for the language ("Python") it does not provide the language specific suggestions.

Perhaps I am missing a process in running the system? To see a change do I just need to run make serve or are there any steps before that I need to take to allow for the system to make use of the new change?

[M-Zuber]

The test and implementation look great! As far as running the system, I have not really worked on the web side that much, but I believe running make compile before make serve should be enough.

Thank you for all the effort you are putting into this.

[ExcaliburZero]

I attempted run make compile, however I am getting some errors when doing so.

browserify lib/app.js | uglifyjs -c > dist/forkability.edge.min.js
/bin/sh: browserify: command not found
/bin/sh: uglifyjs: command not found
makefile:18: recipe for target 'compile' failed
make: *** [compile] Error 127

I figured that the errors were just due to me having not installed browserify and uglifyjs through npm, so I installed those two modules. However after running make compile again I still had the same errors.

I looked into the error a bit and found that I might be able to get the compile makescript to work by putting npm bin in from of each of the unfound commands. So I did that and running make compile seemed to work. However, when I run the web app with make serve and attempt to enter the project information I have some odd issues.

When I go to enter the language name the button that would normally pop-up with the name of the entered language does not appear, and when I click on the "Check Forkability" button the page just refreshes.

[M-Zuber]

Okay I will have to look into this some more, but in the morning - it's 23:50 by me.

[M-Zuber]

Disclaimer: I am aware of the date, but if I don't write now I will forget. Happy holidays! From what I can tell, the Web page looks at the .edge.js file - which is not created by make compile. So for testing purposes I suggest changing the file loaded in index.html. @basicallydan can correct me if I'm wrong - either way this process needs to be clarified /documented.

[basicallydan]

@M-Zuber @ExcaliburZero Hey guys!

So, first off - great issue and well spotted. Thanks for working on this.

make compile should generate the .edge.js file by default as well as .edge.min.js - that's basically just meant to be the very latest code. If you specify ENV version=0.18 beforehand it'll generate a versioned compiled Forkability. I usually do both of these things before pushing for a new release.

As for the weird errors @ExcaliburZero I can probably take a look at your fork maybe this evening (it's 8AM here) but right now I'm gonna go and do Christmas with my family. In the meantime, if you happen to not be doing Christmas and want to carry on what I'd suggest is that it sounds like there's a JS error on the page somewhere. Open up Dev Tools in Chrome and look for any errors. There may be an error caused by some changes you made which only show up after concatenation & minification (i.e. what Browserify and Uglify are doing).

Finally adding uglify and browserify by dev dependencies and modifying the scripts is probably a good idea to make the whole thing more free-standing!

Thanks again! :christmas_tree:

[ExcaliburZero]

I think I may just have something wrong with the way I'm testing out the changes I made (issues compiling changed source).

@basicallydan Could you take a look at my fork and test it on your end to see if it works?

[basicallydan]

Hey Christopher - yes I can do that. Gimme a few hours, a day max, and I'll get back to you. On 5 Jan 2016 02:38, "Christopher Wells" notifications@github.com wrote:

I think I may just have something wrong with the way I'm testing out the changes I made (issues compiling changed source).

@basicallydan https://github.com/basicallydan Could you take a look at my fork and test it on your end to see if it works?

— Reply to this email directly or view it on GitHub https://github.com/basicallydan/forkability/issues/82#issuecomment-168875623 .

[basicallydan]

@ExcaliburZero Looking at this now :)

[basicallydan]

@ExcaliburZero I can't see any problems with the web app in terms of errors. I was able to do a simple make compile to compile a new version of forkability.edge.js.

As well as that, the changes you made seem to work just fine. Can you describe to me the process you took to test it on the forkability web app, again? did you use make compile?

[ExcaliburZero]

Here is the process I am trying to be able to see the changes in the local run of the web app:

• Use Git to checkout the branch that has the changes.
• Go to the main directory of the project.
• Run make compile
• The following error is then outputted:

browserify lib/app.js | uglifyjs -c > dist/forkability.edge.min.js
/bin/sh: browserify: command not found
/bin/sh: uglifyjs: command not found
makefile:18: recipe for target 'compile' failed
make: *** [compile] Error 127

[M-Zuber]

Ok, so after some playing around and searching I have the following conclusions: browserify (and most probably uglifyjs) need to be installed globally to work.

I would like to commit a change to the make file that will simply try to install them before compiling. This should work in almost (if not all) all scenarios. See #87 @basicallydan what say you?

[M-Zuber]

@ExcaliburZero once #89 is merged, update your fork from this repository and things should work.

[ExcaliburZero]

I have updated my fork and feature branch with the latest changes to the upstream repo, and ran make to install the dependencies. However, when I ran make test the test that I setup for testing uppercase names failed with the message Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

Then when running make compile and then make serve inputting an uppercase language name "Python" does not work, while inputting a lowercase name "python" does work properly.

I am not sure what is causing this issue, as even the console log message that is given when an invalid language is entered uses the capitalized version despite the fact that I set the message to use the lowercase version. Perhaps I am missing something simple here.

Sorry, Forkability doesn't support the language "Python. If you're keen you can open a pull request at https://github.com/basicallydan/forkability/issues"

Can someone take a look over it? Here is the branch I am working on: https://github.com/ExcaliburZero/forkability/tree/lang-case

[M-Zuber]

Can you open a PR from that branch? It will make it easier to give feedback on specific items.

In the meantime your test was failing because it was missing

{
  path:'CODEOFCONDUCT.md'
}

in the file set of the mock, After adding that, the passes length should be equal to 14 and not 13

[M-Zuber]

Can you also update the Makefile to have quotes like so:

compile:
    "./node_modules/.bin/browserify" lib/app.js | "./node_modules/.bin/uglifyjs" -c > dist/forkability.$(VERSION).min.js
    "./node_modules/.bin/browserify" lib/app.js > dist/forkability.$(VERSION).js

On windows it only works with the quotes, but I am not sure about other OS So if it works, I would prefer it to be setup for crossplat

[basicallydan]

@M-Zuber Ack, sorry about that. My unix-centric worldview has tainted my former Windows-centric worldview. :disappointed:

[ExcaliburZero]

I opened a pull request with the changes I put together at #90.

[M-Zuber]

@M-Zuber Ack, sorry about that. My unix-centric worldview has tainted my former Windows-centric worldview. :disappointed: @basicallydan I'll do a quick PR on this

[M-Zuber]

@ExcaliburZero thank you