-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Improve Examples menu #5178
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
Improve Examples menu #5178
Conversation
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-5178-BUILD-582-linux32.tar.xz ℹ️ The |
Looks good Paul. I tried this on my system which has a rather odd multi-sketchbook layout now to keep the core-specific library matching gremlins at bay :) The end result was rather good still... I especially like how the examples specific to the AVR core are grouped together. The PR also seems to fix an issue with some libraries not being put in the custom group where they belong (ie. the Adafruit Nokia library should have been in the examples from custom libraries grouping, which it now is). |
platformName = targetPlatform.getPreferences().get("name"); | ||
platformLibraryPath = new File(targetPlatform.getFolder(), "libraries"); | ||
String core = BaseNoGui.getBoardPreferences().get("build.core", "arduino"); | ||
if (core.contains(":")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there existing code to handle finding out the referenced platform? Or is that removed and moved into arduino-builder now? This would seem like the wrong place to have this logic, perhaps moving it into a Platform method might be more appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If TargetPlatform has a function to get the referenced platform within its API, I agree it should be used rather than parsing the build.core pref. Likewise for finding the human-readable name of the platform. But I didn't see anything like this inside TargetPlatform. I'd be happy to change this to use an official API... I just need someone to tell me which API does this?
Looks good to me. I added some nitpick comments to the code, but overall the code looks great. |
I added another commit to fix 1 of the 3 nitpicks. :) On the other 2, happy to edit, but I need some guidance. If there isn't an API for finding referenced platforms or getting the human readable names (I didn't see any such thing), I believe adding such an API ought to be a separate pull request, which would update this and other places within the IDE. |
I haven't looked to see if there is any code for this, I can imagine that this is no longer needed in the IDE with arduino-builder. If adding such an API, it might as well be a separate commit within this, pullrequest instead of a separate PR, IMHO. However, I don't think this should block this pullrequest, I guess it's perfectly acceptable is it is, it can be moved later if needed. |
Sorry if I'm a little burensome, but I don't know why TFT, GSM and (maybe) Ethernet libraries are still displayed as not "retired". Let's clean up! |
@q2dg - Did you try running the test build? Do you have any opinion about this change to the Examples menu? Or is your only input here which libs are retired? |
@PaulStoffregen Sorry, I'm waiting to a nightly build to test it I didn't want to polute this thread. My only input was about asking for clarify which official libraries have future and which no. I've written several issues here about this topic because for me is very important that information showed in web and in boards, libraries and examples menu showed in IDE is updated. If not, the impression for a newbie is chaotic. Thanks a lot for your work. |
@q2dg - No need get a nightly build. There's a test build right here. Just scroll up. It's the 2nd message, from ArduinoBot. However, this pull request is 5 days old, so the nightly build should certainly have it by now. Really, there shouldn't be any reason you can't actually try this, if you wanted to contribute in a productive way. I know you wish to talk about changing the properties files within some libraries, but that's outside the scope of this work. This pull request is about improving the Examples menu code. As a simple matter of correctness, the code is supposed to show the libraries marked retired in the RETIRED sub-menu, and the others normally, according to the actual libraries properties data. This code is not supposed to guess which libraries ought to become retired or otherwise disregard the actual library properties data. |
Last nightly build is from 26th July. I'm not developer, I'm just an user which takes care of the clarity and userfulness of documentation for users like me. You're right this was not the place to ask for deprecate libraries/examples related to retired products. Sorry. |
No, the nightly is a build of the master branch, so it will include this PR only after merging. |
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-5178-BUILD-585-linux32.tar.xz ℹ️ The |
I haven't been following this closely, but I did have a question. Does this show examples from library versions that won't be used for compilation (because they're being overridden by a library of the same name in a higher-priority location)? If so, that seems confusing. To me, it seems like it would be better to only show examples from library versions that will be used. |
No, it does not show those unusable libs, because it gets the list of all libraries from librariesIndexer.getInstalledLibraries(). Of course, this depends on librariesIndexer matching what arduino-builder actually does. Whether the Java & Go code perfectly implement the same algorithm is a good question. But if they don't perfectly agree, that's an issue with librariesIndexer. |
That would be a useful distinction to make (though I wouldn't hide unused version but mark them as such), but also tricky to implement. In part due to the java vs go distinction, but that might be solved by calling arduino-builder to sort these things out. But more importantly, the question of what library will or will not be used, depends highly on circumstances. If two libraries share one header file but also have another, different header file, which library is used depends on which of these header files are actually included. |
Perhaps these are 2 different questions? The builder will use specific libraries depending on the include lines it discovers, which depends on the compiler's preprocessing which takes conditional compilation into account. The librariesIndexer in Java is concerned with the simpler question of all the libraries the builder could possibly use. The examples menu has always offered all the libraries which could be used. It's always hidden the redundant copies of libraries which can't actually be used. This patch preserves that long-standing design, which I believe is good. It merely reorganizes the list to give a better visual cue, so the end user can see whether the Servo library they're using will be the one that's built into Arduino, or a copy provided by the platform, or one they've installed which is overriding both of those. |
@cmaglie - How do you feel about merging this for 1.6.11? Too risky? |
@PaulStoffregen |
@PaulStoffregen I want to release 1.6.11 ASAP and this would block it again. My apologies for the back and forth. I'll follow this up closely after 1.6.11 and make sure it go into 1.6.12. |
Ok, that's fine. There really no hurry. |
/cc @00alis |
@ArduinoBot build this please |
Sorry for the late feedback! I think from the UX point of view this works and it is pretty compatible to the Web IDE. Online we have 2 main tabs: BUILT-IN and FROM LIBRARIES, please check it out yourself on create.arduino.cc/editor (you just need a Arduino ID to login). Built-in tab is exactly the same as you show it in the Java IDE; in the From Library tab we just have a Tag for the architecture and we offer the opportunity to filter out the libraries based on the architecture of the board currently linked to the sketch. We also have search embedded in the panel, which is the feature users seem to use the most. We are currently working on the complete restructure of the Library and Example API, it is going to be ready in a couple of weeks. This new release will also include the RETIRED section. Thanks! |
@cmaglie Any chance this can be reconsidered for 1.6.12 ? |
This patch changes the File > Examples menu to show libraries grouped by their type, either built in to the IDE, supplied by a platform, or custom installed.
Currently, libraries are grouped into "Examples from Libraries" and "Examples from Custom Libraries", with little rhyme or reason. This patch attempts to give users a visual cue about the source of their many libraries, so they can understand which are from the IDE, which depend on their board's platform, and which libs are the ones they've chosen to install.
A few corner cases are automatically handled. When the platform references another platform, both sets of their libraries are automatically shown. In the custom libraries, any which are known to be incompatible are shown at the end using a sub-menu, similar to how the retired libs are shown.