-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding an alert when not finding the Test class #4
Conversation
Added an alert to copy test class name if not found in project
Also take some investigation over "Navigate menu issue", but no luck. I've used NSMenuDidChangeItemNotification to log these changes... it seems that it changes dynamically on project/file opening |
Very cool, thanks! I'll take a look at it when I get a chance. I need to get the unit test suite back up and running... also on my todo list! |
After reviewing this I'm going to have to reject it. I have some feedback that will help us both in the future. There are 2 features and 1 bug fix in this pull request in a single commit. I cannot stress enough to group context sensitive changes into separate commits. I see at least 3 here:
If you would like to open a pull request for the bug, I'd be happy to merge it. Unfortunately, I'm not interested in adding an NSLog to the plugin--I don't think NSLogging is particularly useful in this situation. For item 2, I have other ideas. There are currently scenarios that exist where I have a test class, but it's not found in the plugin. For example, assume a class ABCDataMigrator.m exists, but the tests for it are so broad I've separated them into multiple files such as ABCDataMigratorModel1Tests.m and ABCDataMigratorModel2Tests.m. This is a real scenario that tools such as AppCode provide an alert with a selection of possible files to jump between. I'd like a feature that behaves similar to that, and when it cannot find the test file then you can optionally associate one, or more, with the source file. |
Ok, so we need to add more buttons to the alert like :
Maybe I can implement this and send another PR ? Ok for NSLogs, I like to got them with a "dev/debug" version because we can't debug the plugins (last time I tested, got a crash when Xcode spawns another Xcode). But unit tests are here for that :-) |
As for the "alert", I'm thinking more along the lines of a dropdown selector; see the images on FuzzyAutocomplete and Code Pilot for what I'm thinking--I'm not interested in a plain alert view with buttons.
I'm not sure what you mean by that statement. I don't think there's any advantage to searching for recently opened files. Xcode knows which files are test files, and it's not terribly difficult to match up a test file with it's main counterpart at least the majority of the time.
If the plugin isn't working then it's a clear indication that you need to launch Aviator within Xcode to start debugging. You're free to do what you want in development, such as scatter NSLogs around or enable Xcode's All Exceptions breakpoint. I just ask that you please not commit these into the repository. I'm very picky about code 😊 I appreciate the interest you've taken! |
Hmm, seems there is three cases :
For the first, the "NSAlert" looks good to me, the second is already implemented. And the third need a "custom UI", I tried a simple NSMenu : Forgot "to clipboard" in the third choice. Esc allow to leave this menu, but this menu is only "mouse", no up/down arrows to select the choice (maybe it can be tweaked). What needs to be changed is to have "referenceCollectionForSourceCodeDocument:" return an array of TFFFileReferenceCollection. |
I definitely do not want an NSAlert telling me there are no tests. We can do better. We should have the ability to directly open the XCTest File Template and allow the user to create the test file.
I'm not sure NSMenu is the proper UI component for this behavior; I would think an NSView that holds a list of selectable elements would be prime. See my previous comment with examples and pictures.
I'll give it some thought. That may be appropriate. |
Sure, but the issue I had is that we can't know in which group to put the newly created file (aside the "under test file" like you do, or in the "BundleTests groups". Then the second issue is to "simulate user input to create the new file", so the easy way for me is to have the test class name copied, and then the user can easily right click into the group he wants the to be (and maybe create this group just before).
Sure it is not the best, but it can be added very quickly, and then see if the "not so often" usage of this menu still requires a better UI. |
I see what you mean, and it's a fair point. I think with a first pass it can default new test files to be added to the Tests group, since that probably aligns with most developers--I understand my style is unique. If that were the case, it would be great in the future to add a preferences window to be able to specify where default tests were created, if alerts should be shown, etc. Also, if there is an alert view, I like how AppCode presents it. It's not in your face and it is automatically hidden after a about a second of delay.
Not sure why you think we'd have to simulate user input to create a file. We have the entirety of Xcode's private API at our fingertips! 😄 👍 It should be totally possible. Also, I want to stress that this is not intended to be easy, I mean look at the dance I went through to scrap through Xcode's private headers just to get this far! 😨 But what I am saying is that nearly anything is possible.
Ok, I'm not 100% against it, it just feels like it's improperly being used. I don't want you to feel pressured into doing this if it sounds overwhelming. I'm capable of implementing these features and would gladly do them. If you want to contribute some to this idea--please do--but just be aware that I tend to be hypercritical 😉. It may be a good idea to break these up into different issues even. |
Sure the better the UI is, the most it will be usable. Maybe there is two or three days of work for making this feature. The great thing is that AppCode is making Xcode better (and vice-versa). Having access to some private "APIs" is also very interesting to add such feature to Xcode. I don't know so much Mac APIs (iOS primary), but would you consider PR to a feature branch to implement these features ? |
👍
I'm in no rush to get any of these features in. If it takes 3 days or 3 weeks, that's fine. The main thing I'm concerned about is code quality and a solid, functioning plugin.
There's more of an onus for Xcode to improve, that's for sure!
Yes, I hope you don't think Xcode plugin architecture is documented, because it's not. It's all one giant hack, it kind of sucks. This is a good article on what goes into building Xcode plugins. Here is a dump of the private headers. It's a little out of date, but that's what I've been using for reference.
I will open some new issues with some detail regarding what we've discussed in here. I think it's all good information. If you want to work on a feature branch and let me know, I'd be happy to provide feedback. But as I stated, Xcode plugin development is a tricky beast. It's a lot of hacks strung together and requires reading a lot of reading of private headers and it can be painful to debug certain things in order to make them work. I really think this is a very involved task that will take many many days to get 100% complete. I also appreciate unit tests written where they make sense (when working in private Xcode API land). If that doesn't interest you, then I'd consider accepting pull requests for the alert/menu, preferences pane, etc. Just keep me updated on what you do. If you do decide to work on some of this, I would like to give feedback along the way so that you don't end up wasting a lot of time. 😄 |
Thanks, I've used some plugin mentionned (XcodeExplorer) to made some internals plugins in my company (that should be pushed to github, but there are more proof of concept when I wrote them, so code quality is not so high...) I've seen the Ctrl-R button from #5 to launch tests for selected file(s) : "wahou", very interesting feature... I'll post on issue before trying some dev on it (and I'll made a more "unit" commit this time). Regards |
Delayed menu injection because the Find -> "Jump to Test File" was not appearing
Add some logs in exception that prevents Xcode from crashing
Added an alert to copy test class name if not found in project :