Okay, I did a quick review:

The CSSParameter additions look good.

With regards to the parameter replacement - I think there's probably a more
concise way of doing that, but I don't know what it might be, and what you
have there does seem to be both stable and ultimately correct.
I did find one issue - if you have a new or empty file, the namespace
substitution code throws an error, causing all autocompletion to fail. I
have pushed a fix.
Other than that, the actual functionality seems great.

One other thing I did notice was a "test.js" file was added, which appears
to be a duplicate of the tags - this seems like it is a temporary/"backup"
file, and should eventually be removed?


> I believe the completion could work better in a couple of cases:
>
>    - If the element being completed has no attributes, complete it
>    "closing" it (e.g. do not complete "<Stroke" but "<Stroke>", while keeping
>    "<CSSParameter" or "<ExternalGraphic" as is)
>    - When adding the closing tag it should be obvious which tag should be
>    completed when "/" is pressed, complete directly without proposing
>    alternatives in that case
>
> While I was taking a look at your changes, I also took a quick look at
these two as well and have pushed a couple of commits that add these
capabilities.
I've pushed all my changes. Seems like we might be getting close to
something that is mergeable, or at least ready for a 3rd-party review/PR.

Torben


> Also, we could have completion support for SLD 1.1 as well. I'm going to
> have a look at these topics and report back.
>
> Cheers
> Andrea
>
>
> On Wed, Sep 26, 2018 at 1:30 AM Torben Barsballe <
> [email protected]> wrote:
>
>> I've pushed up some potential improvements to how the autocomplete
>> triggers.
>>
>> Rather than trigger after certain keypresses, autocomplete will now check
>> after any keypress (I have included a blacklist, so we can exclude any keys
>> as needed). To make this actually useful, I've added some additional checks
>> based on the cursor position so it will only show the autocomplete menu if
>> it would be useful to do so, in particular:
>>   - If the cursor is currently on an empty line
>>   - If the cursor is currently in a tag, and either at the end of the tag
>> name or at the end of an attribute
>>
>> Feel free to try it out and see what you think. I think it makes it a
>> little less clunky, without being too naggy.
>>
>> Torben
>>
>> On Tue, Sep 25, 2018 at 9:21 AM Andrea Aime <[email protected]>
>> wrote:
>>
>>> On Tue, Sep 25, 2018 at 6:08 PM Torben Barsballe <
>>> [email protected]> wrote:
>>>
>>>> Awesome. Should I push it to a branch on GeoServer, so that we're both
>>>>> able to commit directly on it?
>>>>>
>>>>
>>>> Yes, that sounds good. Thanks!
>>>>
>>>
>>> There you go: https://github.com/geoserver/geoserver/tree/sld10-hint
>>>
>>> Cheers
>>> Andrea
>>>
>>> ==
>>>
>>> GeoServer Professional Services from the experts! Visit
>>> http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf
>>> Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa
>>> (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549
>>> http://www.geo-solutions.it http://twitter.com/geosolutions_it
>>> ------------------------------------------------------- *Con
>>> riferimento alla normativa sul trattamento dei dati personali (Reg. UE
>>> 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si
>>> precisa che ogni circostanza inerente alla presente email (il suo
>>> contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è
>>> riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il
>>> messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra
>>> operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
>>> This email is intended only for the person or entity to which it is
>>> addressed and may contain information that is privileged, confidential or
>>> otherwise protected from disclosure. We remind that - as provided by
>>> European Regulation 2016/679 “GDPR” - copying, dissemination or use of this
>>> e-mail or the information herein by anyone other than the intended
>>> recipient is prohibited. If you have received this email by mistake, please
>>> notify us immediately by telephone or e-mail.*
>>>
>>
>
> --
>
> Regards, Andrea Aime == GeoServer Professional Services from the experts!
> Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
> @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
> Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
> 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
> ------------------------------------------------------- *Con riferimento
> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
> circostanza inerente alla presente email (il suo contenuto, gli eventuali
> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
> sarei comunque grato se potesse darmene notizia. This email is intended
> only for the person or entity to which it is addressed and may contain
> information that is privileged, confidential or otherwise protected from
> disclosure. We remind that - as provided by European Regulation 2016/679
> “GDPR” - copying, dissemination or use of this e-mail or the information
> herein by anyone other than the intended recipient is prohibited. If you
> have received this email by mistake, please notify us immediately by
> telephone or e-mail.*
>
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to