imagej / imagej-legacy

ImageJ+ImageJ2 compatibility layer
https://imagej.net/libs/imagej-legacy
BSD 2-Clause "Simplified" License
16 stars 25 forks source link

Macro autocomplete enhancements #225

Closed ndefrancesco closed 4 years ago

ndefrancesco commented 4 years ago

Hello everyone! Hi @imagejan and @ctrueden,

I've been making some enhancements to the macro autocompletion of user-defined variables and functions, which have been posted in the last few days in the image.sc auto code completition post.

I've tried my best at testing the code, and it seems to behave as expected. @haesleinhuepf has provided valuable feedback (and guidance!) in this process.

The changes introduced can be summarized as follows :

As many of you probably know, I am a complete novice at doing this kind of process, including filing this very same first-ever PR. In other words, I don't know much about this process or what to expect, so I'll likely learn along.

I'll leave it to your inspection and consideration. All your inputs will be most valued!

I look forward to your comments.

Cheers! Nico

ndefrancesco commented 4 years ago

A couple of final questions, as I am still trying to understand the dynamics of collaborative coding:

At the moment I have some more changes in mind for this functionality, and some of them are more profound than others. Where's the appropriate place to discuss some ideas and to agree on the most sensible way to proceed, and perhaps on what endeavours it is best to invest efforts in? I suspect @haesleinhuepf should mainly be involved in this kind of conversation, but I haven't figured out where it should take place.

Also, if some of these changes are simple to implement and somewhat related to this PR ( e.g. further refining of the infor provided in the tooltips, the formatting, etc. ), is it preferrable to wait some more time before merging and add them to this same PR, or is it better to just release in this state, and work on a second PR?

Thanks for the guidance!

ndefrancesco commented 4 years ago

In the meanwhile I added a few extra things:

haesleinhuepf commented 4 years ago

Hey @ndefrancesco ,

I think you did everything right. Also the changes you just introduced sound really good! The only minor comment I'd like to suggest is: If you send a pull-request, please be sure that everything is complete and finished. By adding more changes later on, you may increase workload on the other side (Some of the core-maintainers are really busy people). Furthermore, I always found discussions on the forum helpful with a higher-level picture: What do users need? What's important? What should have priority? What do testers like? What doesn't work from their perspective? Technical detail discussions are maybe better done on github. For real-time chats on technical details gitter.im is helpful. I will just download your recent code, put it in my ImageJ and work with it for a couple of days. Just to ensure that there is no glitch in the script editor. Highest priority is not breaking anything when updating core parts of ImageJ/Fiji ;-)

Thanks again for your contribution! I keep you posted!

Cheers, Robert

ndefrancesco commented 4 years ago

Thanks, @haesleinhuepf, and sorry for the overloading. I was unsure about adding those extra commits for that same reason. I'll be more careful about this in the future. Perhaps a revision of recently closed PRs in this repository might help me get a better sense of the normal dynamic.

As for the user input, I fully agree that it is key to directing feature development and to help detecting bugs early (that's usually been my side 😄). Besides from your (much appreciated!) personal testing, I'll make a post in the forum with the latest version, in case anyone else might want give it a try and report back.

I'll wait for your comments and feedback. Thanks again!! Nico