Re: [code] D Language plugin for Textadept (Now with Windows support)

From: Brian Schott <briancschott.att.gmail.com>
Date: Thu, 8 Jan 2015 22:46:26 -0800

Thanks for the code review. I fixed up the autocomplete image registration
code and server auto-starting.

Want to see something funny? Both of us missed this:
https://github.com/Hackerpilot/textadept-d/blob/c6f0a2b6e03816bf1c5213362c0306d0118f78fc/dmd/init.lua#L16-L17

On Thu, Jan 8, 2015 at 7:03 AM, Mitchell <m.att.foicica.com> wrote:

> Hi Brian,
>
>
> On Tue, 6 Jan 2015, Brian Schott wrote:
>
> https://github.com/Hackerpilot/textadept-d
>>
>> This module provides autocomplete, syntax checking, and code linting for
>> the D programming language in Textadept. (Some of these features have
>> existed for a while across the DCD repository and my TextadeptModules
>> repository, but now they're all in one place and working on Windows)
>>
>
> Nice work! I have a couple of comments in `dmd/init.lua`:
>
> 1. In `registerImages()`, you are calling `buffer:register_image(n, ...)`
> where n = 1, 2, etc. This happens to overwrite Textadept's built-in XPM's
> (from modules/textadept/editing.lua). The recommended practice is to use
> `_SCINTILLA.next_image_type()` to get a unique image type. I know you
> depend on those constants later in `showCompletionList()`, but I'm only
> bringing this up because users who happen to use other languages that have
> autocompletion might find it odd that a list's images no longer line up
> with their associated content.
>
> 2. In the `events.QUIT` handler, you are comparing a process' status
> against the variable `running` when you might have meant to use the string
> `"running"`. Also, you spawn a command to shutdown your DCD client and then
> immediately kill your server process if it's still running. I'm not
> familiar with your architecture, but if the call to spawn is supposed to
> cause the server process to stop itself, then you should probably wait for
> a brief moment in time (via `_G.timeout()`) to allow the server to get
> client notification and handle a graceful shutdown since `spawn()` is
> asynchronous and pretty much returns immediately.
>
> Cheers,
> Mitchell
> --
> You are subscribed to code.att.foicica.com.
> To change subscription settings, send an e-mail to code+help.att.foicica.com.
> To unsubscribe, send an e-mail to code+unsubscribe.att.foicica.com.
>
>

-- 
You are subscribed to code.att.foicica.com.
To change subscription settings, send an e-mail to code+help.att.foicica.com.
To unsubscribe, send an e-mail to code+unsubscribe.att.foicica.com.
Received on Fri 09 Jan 2015 - 01:46:26 EST

This archive was generated by hypermail 2.2.0 : Fri 09 Jan 2015 - 06:37:06 EST