On Mon, Dec 19, 2022 at 1:40 AM Mikhail Gribkov <youzh...@gmail.com> wrote:
> Hi Ted, > > > bq. in to the system > > 'in to' -> into > > bq. Any bugs in a trigger procedure > > Any bugs -> Any bug > > Thanks! Fixed typos in the attached v35. > > > + if (event == EVT_Login) > > + dbgtag = CMDTAG_LOGIN; > > + else > > + dbgtag = CreateCommandTag(parsetree); > > The same snippet appears more than once. It seems CMDTAG_LOGIN handling > can be moved into `CreateCommandTag`. > > It appears twice in fact, both cases are nearby: in the main code and > under the assert-checking ifdef. > Moving CMDTAG_LOGIN to CreateCommandTag would change the CreateCommandTag > function signature and ideology. CreateCommandTag finds a tag based on the > command parse tree, while login event is a specific case when we do not > have any command and the parse tree is NULL. Changing CreateCommandTag > signature for these two calls looks a little bit overkill as it would lead > to changing lots of other places the function is called from. > We could move this snippet to a separate function, but here are the > similar concerns I think: it would make sense for a more common or a more > complex snippet, but not for two cases of if-else one-liners. > > -- > best regards, > Mikhail A. Gribkov > > e-mail: youzh...@gmail.com > *http://www.flickr.com/photos/youzhick/albums > <http://www.flickr.com/photos/youzhick/albums>* > http://www.strava.com/athletes/5085772 > phone: +7(916)604-71-12 > Telegram: @youzhick > > Hi, Mikhail: Thanks for the explanation. It is Okay to keep the current formation of CMDTAG_LOGIN handling. Cheers
