Fix indexing error - Githubissues

Korosensei42 commented 5 years ago

This should fix #50.

The issue was, that an input like "ff ffff" spat out four issues when only two shoul be created.

The reason of this was, that "ff" was detected as an invalid word and the whole input was searched for additional occurences of "ff".

The solution I implemented was: A word is classified as some sequence of characters between whitespaces. Therefore I now take the user input and append a Whitespace in the front and back of it and only look for occurences of " ff ", i.e. occurences of "ff" that have a whitespace in the front and back now. This solution is a bit hacky, but it should be justified, since it builds upon the definition of a word.

TimothyGillespie commented 5 years ago

Seems to fix the issue as I couldn't reproduce it.

I will approve this when

the new test cases for this error will be added (and maybe also the previous one for $Sonne Sonne$ which I've forgotten last time).
the JSDocs with the @function keyword will be edited or reasoned why the keyword is used correctly here.
the reason for the issue could be described very briefly and how it was fixed. (If this is easily describable in no more than three sentences)

Korosensei42 commented 5 years ago

@TimothyGillespie I think I adressed everything you asked for. For a more detailed explanation see the edited description at the top. Thanks for your feedback. If there are still some problems you see, don't hesitate to tell me.

TimothyGillespie commented 5 years ago

Looks good so far. I think there has been a misunderstanding the dash in the JSDocs. I think it should be there. Not that it was inconsistent. Now I am not sure if it is really important. I just always see the dash put that way and it seems standard. But since JSDocs will/could be parsed later a missing dash could break it? Just in case.

I noticed one more thing which is unrelated to this specific issue. That is in line 128 in the tests. It should check for the input a space. But currently it only empties the list and then confirms that the issue list is now empty.

Anyway: good job.

If you could take a look at the test in line 128 and either explain or correct it I will give the approval for the PR.

Korosensei42 commented 5 years ago

@TimothyGillespie That most likely was a copy-paste error from some other time. I corrected it.

diproche / webinterface

Fix indexing error #83