ray-x / go.nvim

G'day Nvimer, Joyful Gopher: Discover the Feature-Rich Go Plugin for Neovim
MIT License
2.06k stars 123 forks source link

feat: improve queries #316

Closed amaanq closed 1 year ago

amaanq commented 1 year ago

Hello, I'm a treesitter maintainer for nvim-ts and a bunch of ts grammars, I use this plugin to enrich my (admittedly sparse) go development. I noticed the queries are a bit expensive and not as optimized as they could be for the end user, so I tidied it up and made some changes. Note that #match? is quite expensive and #lua-match? is much better.

Thanks, Amaan

amaanq commented 1 year ago

Side note, I don't exactly understand the point of the locals query..very weird naming convention there from what I can tell.

ray-x commented 1 year ago

Here is an example

const(
  queryUser = "select * from users where user_id=?"
  queryUserComments = "select * from user_comments where user_id=?"
  queryUserID = "select user_id from users where user_name=?"
)

Someone may prefer this instead

const(
  userQuery = "select * from users where user_id=?"
  userCommentsQuery = "select * from user_comments where user_id=?"
)
amaanq commented 1 year ago

Here is an example

const(
  queryUser = "select * from users where user_id=?"
  queryUserComments = "select * from user_comments where user_id=?"
  queryUserID = "select user_id from users where user_name=?"
)

Someone may prefer this instead

const(
  userQuery = "select * from users where user_id=?"
  userCommentsQuery = "select * from user_comments where user_id=?"
)

Right but query as a word doesn't just mean sql query, it has other meanings too and can be used for other things imo, but I guess for raw string literals only then it can be ok. Match is expensive so contains would be better, lmk if that's ok w/ you

ray-x commented 1 year ago

I agree query is not a good way to detect SQL string. maybe we combine it with keyword search inside strings ; "ADD" "ADD CONSTRAINT" "ALL" "ALTER" "AND" "ASC" "COLUMN" "CONSTRAINT" "CREATE" "DATABASE" "DELETE" "DESC" "DISTINCT" "DROP" "EXISTS" "FOREIGN KEY" "FROM" "JOIN" "GROUP BY" "HAVING" "IN" "INDEX" "INSERT INTO" "LIKE" "LIMIT" "NOT" "NOT NULL" "OR" "ORDER BY" "PRIMARY KEY" "SELECT" "SET" "TABLE" "TRUNCATE TABLE" "UNION" "UNIQUE" "UPDATE" "VALUES" "WHERE"

amaanq commented 1 year ago

Yeah that's a better way to go about it, I'll edit this to include those and remove the var name matching.