Re: [jdee-users] [PATCH 1/2] Factorize minibuffer prompts to read a class name in `jde-read-class-name'.

Paul Landes <[email protected]> writes:

> Ludovic Courtès writes:

[...]

>  > I'd love to see it in the repo, to avoid duplicating work and to know
>  > what nice features are going to land in JDEE.  ;-)
>  > 
>  > Maybe you could commit it in a separate branch to start with, until you
>  > deem it "ready" for trunk/2.4.0?  What do you think?
>
> I'd rather not.

I think having the ability to see WIP would be helpful to contributors
and users (or at least to me).  I'm afraid I don't understand the reason
why you don't want to make it visible for now; can you explain?

> But here's the pertinent code in raw form:

[...]

> (defun jde-cust-select-class (&optional prompt fq-prompt
> 					this-class-p confirm-fq-p)
>   "Select a class interactively.  PROMPT is used to prompt the user for the
> first class name, FQ-PROMPT is used only if the class name expands into more
> than one fully qualified name.

[...]

>  > >  > > Also, the direction I'd like to set is one function for class
>  > >  > > reading/finding/visiting.  Currently we have two, this adds a third.
>  > >  > > I'd like go keep it at one.
>  > >  > 
>  > >  > I agree in principle (that was the whole point of this patch: to share
>  > >  > the read-class-name code between `jde-run' and `jde-debug'), but which
>  > >  > functions are you referring to?
>  > >
>  > > - jde-open-class-at-point
>  > > - jde-open-class-source
>  > 
>  > Hmm, these functions don't seem to have any connection with the proposed
>  > `jde-read-class-name'.  `jde-read-class-name' just asks the user for a
>  > class name, nothing more, the idea being that code interactively asking
>  > for a class name should use it, for the sake of UI consistency.
>  > 
>  > Or did I misunderstand your comment?
>
> Maybe.  Take a look at the code above.  The idea is to have one
> function that takes care of every aspect of getting a class name,
> including optionally trying to complete it.

OK.  My patch does 2 things:

  1. Add a `jde-read-class-name' function, which reads a class name
     interactively.  Your `jde-cust-select-class' does the same job, but
     does it much more nicely, so I'd suggest using it.

  2. Actually use `jde-read-class-name' in `jde-run' when asking for a
     class name.  (In a subsequent patch, `jde-debug' is changed to use
     `jde-read-class-name' as well, for the sake of UI consistency and
     code factorization.)

I can think of two ways to handle this:

  1. Commit `jde-cust-select-class' and then change `jde-run' (and then
     `jde-debug') to use it.

  2. Commit `jde-read-class-name' (possibly renaming it to
     `jde-cust-select-class') and change `jde-run' (and `jde-debug') to
     use it.  Eventually, when your `jde-cust-select-class' is
     committed, update callers.

Which scenario would you prefer?

Thanks,
Ludo'.


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july