Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find and fix of typos/misspellings #1

Closed
wants to merge 1 commit into from
Closed

Find and fix of typos/misspellings #1

wants to merge 1 commit into from

Conversation

TomasRoj
Copy link

@TomasRoj TomasRoj commented May 9, 2019

Hey,

I found and fixed some typos/misspellings in your repo. Hope this will help!

@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

Hi Tomáš,

thank you for looking for typos at my repos, but please do not correct typos in third party libraries (I have not made)! Better ask them to correct it.

Best regards
Armin

@ArminJo ArminJo closed this May 10, 2019
@TomasRoj
Copy link
Author

Hey, I am using @per1234 inolibbuglist with codespell library. the script doesnt know that this lib isnt yours so I cant control. Also the codespell fixes some words that are correct due that I am not englishian I cannot fix themm al correct even I control every fixed word. Have a nice day,.

@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

Ok, now I know the background.
Maybe the script can be improved, since it is very annoying to get pull request for typos, which are in 3. party libraries, which I can and will not change.
Thanks
Armin

@TomasRoj
Copy link
Author

Yes you are right. Plase write if you have any ideas how to do it :-D

@TomasRoj TomasRoj deleted the fix-typos branch May 10, 2019 16:14
@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

Then send me the script :-D

@TomasRoj
Copy link
Author

Its availibe on github - codespell

@per1234
Copy link

per1234 commented May 10, 2019

codespell simply identifies all occurrences of commonly misspelled words in the text. It has no way to know whether the proposed change is a false positive, will break the code, or is otherwise unwanted. This is why it's the responsibility of the person using codespell (@TomasRoj) to carefully review the changes proposed by codespell. codespell takes care of the boring first 95% of the job, but a human must do the essential final 5%.

You can exclude directories from the codespell scan. In this case, you could do:

codespell -w --skip="./src/lib"

I could add that to the inolibbuglist script. The question is whether this is a common convention for Arduino libraries to bundle other libraries under src/lib. What I see usually is that when an Arduino library had a dependency on other libraries, they simply add instructions to the readme that the dependencies should be installed separately.

Then send me the script :-D

codespell is here:
https://github.com/codespell-project/codespell

My inolibbuglist script is here:
https://github.com/per1234/inolibbuglist

I think the change would more likely be made to inolibbuglist than to codespell. However, inolibbuglist is only used to automatically generate a list of Arduino libraries with misspelled words. @TomasRoj is then running codespell manually on each of those repositories to fix the misspelled words. So even if src/lib was excluded from the inolibbuglist scan, @TomasRoj would still need to exclude that folder when they run codespell to fix the mispelled words.

Code contributions or suggestions for improvement to inolibbuglist are welcome.

@TomasRoj
Copy link
Author

Yes

@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

Hi together,
since you are the arduino gurus, one question.
How can I use 3.party librarys im my examples, other than state by text " please install library XY by use of library managen and search for XY... before you run this example" ?
The only way I know is to put them into the example subdirectory. Putting them under src/lib does not work for me as well as putting them in a example subdirectory lib e.g. like examples/BlueDisplaExample/lib.
I just checked it.
If you also do not know another way, then you may consider to run codespell with the -S option and patterns like /lib/ and adafruit and twi.* and so on.

I want to repeat, that it is very annoying for a developer to get pull requests for typos in 3.party libraries, which he is not responsible for and he do not want to change because of their 3.party nature.

And I have to mention, that I appreciate it very much, that you put so much effort in increasing the quality of the libraries, and that I want to support you (if I really can).

Have a nice weekend
Armin

@TomasRoj
Copy link
Author

Hey Armin,

I am glad that I can meet new and positive like people like you on my journey via helping people with open source technologies. I will be happy if we can collaborate in the future for some project. But back to your question. From the start I didn't really understand your issue. You have this repo and you are using 3 party libraries right? Do you have these in some folder in this repo? If yes I recommend not doing this because this causes problems like this. BTW I am not any guru. I am just fan and learning every day.

@TomasRoj
Copy link
Author

Ok I checked one more time. Don't save the libraries itself to repo. It's not your work. Tell he user to install them. This can cause to problems like this but I like your interest to discussion. Don't worry!

@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

Ok Thomas,
but you know the typical arduino user testing a new library like my students (and me)? They install a library, load the examples ans then it should run and not complain, to install this and that libray manually.
Therfore I put all the needed stuff for my examples in the example folder, since other folders do not work.

So I have to live with the typo pull request... but it is better than annoying the users with errors because of missing libraries.

The Arduino IDE should offer a way to put 3.party libraries to a special place, lets say to /src/lib, this would solve our problem. But this is future....
Best regards
Armin

@TomasRoj
Copy link
Author

@ArminJo Yes it future but.. How future is made? By building it... I will try to find more info about this topic and try to find some explanation etc. If there is no way how to do it, we must do it. It's simple 😂

@TomasRoj
Copy link
Author

It's definitely good topic for discussion. If not me, @per1234 will defineteli find a solution.

@ArminJo
Copy link
Owner

ArminJo commented May 10, 2019

This is the right attitude, thank you for this!!!

@per1234
Copy link

per1234 commented May 10, 2019

How can I use 3.party librarys im my examples, other than state by text " please install library XY by use of library managen and search for XY... before you run this example" ?
The only way I know is to put them into the example subdirectory. Putting them under src/lib does not work for me as well as putting them in a example subdirectory lib e.g. like examples/BlueDisplaExample/lib.

It depends on the circumstances. If the 3rd party library is only used by the sketch, then you can put it under the src subfolder of the sketch. For example, let's say you had an example named Foo which had a dependency on a 3rd party library named Bar. You could then do this:

Arduino-RobotCar
|_examples
   |_Foo
      |_Foo.ino
      |_src
         |_Bar
            |_Bar.h
            |_Bar.cpp
            |_etc...

Then in Foo.ino, you can #include the library like this:

#include "src/Bar/Bar.h"

The only problem you might encounter is that some Arduino libraries use the incorrect #include syntax. Let's say Bar.cpp has this line:

#include <Bar.h>

Although not best practices, that will work when the library is installed normally (which is why it's a common issue), but it will not work when the library is bundled with a sketch, since Arduino-RobotCar/examples/Foo/src/Bar is not part of the include search path. The solution is to change the library to use the correct include syntax:

#include "Bar.h"

Which causes the local folder to be searched before the standard include search path.

In the case of bundling multiple libraries that have dependencies on each other, you will need to change the #include directives to use relative paths. For example, let's say you want to bundle a library named Baz, which has a dependency on the Bar library. It would likely have an #include directive like this:

#include <Bar.h>

which you would need to change to:

#include "../Bar/Bar.h>

However, in this Arduino-RobotCar library I see that you are using the 3rd party libraries in the Arduino-RobotCar library itself, not only in the examples. In that case, you can not use the 3rd library bundled in the src subfolder of the sketch and you must bundle the 3rd party library with the Arduino-RobotCar library. There is a feature request to add a libraries subfolder of the sketchbook, which would be part of the include search path: https://github.com/arduino/Arduino/issues/7755

I want to repeat, that it is very annoying for a developer to get pull requests for typos in 3.party libraries, which he is not responsible for and he do not want to change because of their 3.party nature.

I completely understand your frustration. The majority of Arduino libraries are written by people purely as a volunteer labor of love. The theory is that the Internet makes it possible to freely share digital content with everyone in the world. Maybe I wrote the code for myself, but why shouldn't I publish it online so that everyone else can benefit from my effort also? After all it costs me nothing, right? The reality is that this can end up being a burden as the support requests, invalid issue reports, and invalid pull requests start to roll in. Most programmers like writing code, but very few enjoy maintaining a bug tracker. Thus, each invalid pull request is another nail in the coffin of open source. We all have other things we can be doing with our time. If the demands of maintaining a free open source project get to the point where it's not enjoyable any more or is occupying too much of a programmers time, they will likely end up abandoning the project, and perhaps open source altogether. This has happened many times and has been the death of many wonderful projects.

So I do apologize for any part I have played in this situation. However, I want to make it clear that my inolibbuglist project only generates a list of Arduino libraries that might have a common bug or issue. It does not fix bugs. It does not submit pull requests. I wrote this script for my own use, but decided to publish it online in the spirit of open source. I don't think it's my responsibility how others use this information.

I've been trying to provide assistance to @TomasRoj, just as I do for anyone in the Arduino community who asks for my help with something that's within my abilities. I'm sure @TomasRoj would agree that I've been quite a nag to them repeatedly about the importance of quality pull requests and how we need to be very careful that our efforts to contribute result in a net benefit.

The fact is that nobody is perfect. I've accidentally submitted some invalid pull requests myself. I make a very earnest effort to provide helpful pull requests, but in the course of submitting over 4400 pull requests, I was bound to make some mistakes. In the big picture, even someone who only submits valid pull requests 99% of the time is clearly having a net benefit to the open source community. But for that maintainer who only sees the 1 in 100 invalid pull request, it's a 100% failure rate.

So as contributors I think we need to make our best possible effort, but as maintainers I think we also need to try to be understanding.

@per1234
Copy link

per1234 commented May 11, 2019

but you know the typical arduino user testing a new library like my students (and me)? They install a library, load the examples ans then it should run and not complain, to install this and that libray manually.

Work is underway to add a dependency resolver to the Arduino Library Manager. The way it works is you add a requires field to your library's library.properties with a list of the names of the library dependencies. When someone installs your library, the Library Manager will offer to also install the dependencies via Library Manager. This does require that the dependencies are also in the Arduino Library Manager index:
arduino/Arduino#8600

@TomasRoj
Copy link
Author

This last message would will best for you @ArminJo I think.

@TomasRoj
Copy link
Author

TomasRoj commented May 11, 2019 via email

@ArminJo
Copy link
Owner

ArminJo commented May 11, 2019

Work is underway to add a dependency resolver to the Arduino Library Manager. The way it works is you add a requires field to your library's library.properties with a list of the names of the library dependencies. When someone installs your library, the Library Manager will offer to also install the dependencies via Library Manager. This does require that the dependencies are also in the Arduino Library Manager index:
arduino/Arduino#8600

OK, this will help me and others a lot on documenting and releasing a library!!!

For my libraries I do not have simple examples, I also add some of my projects I build this library for, as examples, and they need example specific libraries.
But you gave me the right hint:

It depends on the circumstances. If the 3rd party library is only used by the sketch, then you can put it under the src subfolder of the sketch. For example, let's say you had an example named Foo which had a dependency on a 3rd party library named Bar. You could then do this:

|_examples
   |_Foo
      |_Foo.ino
      |_src
         |_Bar
            |_Bar.h
            |_Bar.cpp

This is an undocumented feature of Arduino library
I testet it with a directory name of lib which IMHO is clearer than src but it did not work, it seems that it must be src.
Who can document this for the next library builder in the Arduino library specs? E.g. like this:
"The source code found in the example src folder ???and all its subfolders??? is compiled and linked in the example sketch. Only the src folder is added to the include search path, so you must reference any include to files in the example src folder in your example with #include "src/<myrequiredinclude>.h" .

But back to the beginning...
Then we may have a chance to filter every example/*/src/ filename with the -S option of codespell.
And to everyone who complains about "strange" pull request, we can give the hint to put his third party stuff to the src/ folder :-).

OK this are my ten cents to this topic.
Cheers
Armin

@TomasRoj
Copy link
Author

Yes, good idea. If you want you can join us by running theese scripts on your computer. Send me your email if you are interested.

@ArminJo
Copy link
Owner

ArminJo commented May 11, 2019

Thank you, but I have a lot to do with my libraries, especially with my never seen before "ServoAsEncoder" one ;-) and this robot car stuff.

@TomasRoj
Copy link
Author

Ok, can I help you with something in theese?

@ArminJo
Copy link
Owner

ArminJo commented May 11, 2019

OK I forgot one crucial aspect:
I like to have the same source for my development and for the examples.
But the incude statement is different, since for development you extend the include path in order not to use #include "src/...h" but just use #include "...h" like it is needed if I install the library before.
The problem can only be solved by extending the Arduino include path (to all lib folders), all the other thoughts are merely dirty workarounds. and will not even work for me :-(.

It was a interesting discussion, but I think Arduino must support this and also a way of using compiler symbols for library examples urgently needed by professional libray developer (like me ;-)). Without this feature there is only the workaround of converting a *.cpp file to a *.h file like here set the symbols before the include statement like here.
But I understand, that Arduino must be simple to use even for contributors.

@ArminJo
Copy link
Owner

ArminJo commented May 11, 2019

Ok, can I help you with something in theese?

Thank you very much, but this is research with servo electronics and for the robot car you need the hardware, but then it might be fun.

@TomasRoj
Copy link
Author

Thanks, I will try to discuss this topic in the official arduino repo too..

@per1234
Copy link

per1234 commented May 11, 2019

I testet it with a directory name of lib which IMHO is clearer than src but it did not work, it seems that it must be src.

That's correct. Recursive compilation is restricted to the src subfolder of the sketch folder.

Who can document this for the next library builder in the Arduino library specs?

This is not about libraries, thus it would not be appropriate to document it in the Arduino library specification. This is about sketches. The recursive compilation of the src subfolder of the sketch folder works the same whether a sketch is a library example or just a sketch by itself. Unfortunately, Arduino doesn't have a sketch specification document. I have proposed its creation to people at Arduino multiple times, but never got any response from anyone on that proposal. There are some other things about sketches that should be documented as well.

I like to have the same source for my development and for the examples.

You should be able to accomplish that with a symlink, or whatever is the Windows equivalent.

The problem can only be solved by extending the Arduino include path (to all lib folders)

Well, I already provided a link to that proposal. You can show your support for it by adding a "thumbs up", but please only comment on the issue threads if you have some valuable information to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants