-
-
Notifications
You must be signed in to change notification settings - Fork 28
feat(add port options): Adding port options and file options to start and pick #322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ if vim.g.loaded_livepreview then | |
| end | ||
|
|
||
| vim.g.loaded_livepreview = true | ||
| local PORT_PREFIX = "++port=" | ||
|
|
||
| local health = require("livepreview.health") | ||
| local cmd = "LivePreview" | ||
|
|
@@ -35,7 +36,30 @@ api.nvim_create_user_command(cmd, function(cmd_opts) | |
|
|
||
| if subcommand == "start" then | ||
| local filepath | ||
| if cmd_opts.fargs[2] ~= nil then | ||
| local port = Config.port | ||
|
|
||
| -- determines if the options given is a port or filepath | ||
|
Ren-B-7 marked this conversation as resolved.
|
||
| for i = 2, 3 do | ||
| local arg = cmd_opts.fargs[i] | ||
| if arg then | ||
| if arg:sub(1, #PORT_PREFIX) == PORT_PREFIX then | ||
| -- If the substring is not a number, this returns nil | ||
| local numb = tonumber(arg:sub(#PORT_PREFIX + 1)) | ||
| if numb then | ||
| port = numb | ||
| else | ||
| vim.notify( | ||
| "Error: LivePreview couldnt parse ++port option. Invalid Number.", | ||
| vim.log.levels.WARN | ||
| ) | ||
| end | ||
| else | ||
| filepath = arg | ||
| end | ||
| end | ||
| end | ||
|
|
||
| if filepath ~= nil then | ||
| filepath = cmd_opts.fargs[2] | ||
| if not utils.is_absolute_path(filepath) then | ||
| filepath = fs.joinpath(vim.uv.cwd(), filepath) | ||
|
|
@@ -54,7 +78,7 @@ api.nvim_create_user_command(cmd, function(cmd_opts) | |
| end | ||
| end | ||
| filepath = fs.normalize(filepath) | ||
| if not lp.start(filepath, Config.port) then | ||
| if not lp.start(filepath, port) then | ||
| return | ||
| end | ||
|
|
||
|
|
@@ -69,7 +93,27 @@ api.nvim_create_user_command(cmd, function(cmd_opts) | |
| lp.close() | ||
| print("Live preview stopped") | ||
| elseif subcommand == "pick" then | ||
| lp.pick() | ||
| -- Create port option, with the default port, then if arguments were | ||
| -- given it checks that it is a valid number and then changes the port | ||
| -- option to the given argument | ||
| local port = Config.port | ||
| -- Must set opt to "" as default else the sub statement returns an error | ||
| local opt = cmd_opts.fargs[2] or "" | ||
| if opt:sub(1, #PORT_PREFIX) == PORT_PREFIX then | ||
| -- If the substring is not a number, this returns nil | ||
| local numb = tonumber(opt:sub(#PORT_PREFIX + 1)) | ||
| if numb then | ||
| port = numb | ||
| else | ||
| vim.notify("Error: LivePreview couldnt parse ++port option. Invalid Number.", vim.log.levels.WARN) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does "Error" use "WARN" highlight?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So in my head i thaught it would have fallback to the basic port option, and still open on the default port if it isnt in use. Which means its not an error, since the program didnt have to terminate early. But when typing it out my head defaulted to It is your call on which you would prefer to change. Either the notification header from |
||
| end | ||
| else | ||
| vim.notify( | ||
| "Error: LivePreview couldnt parse parameter. Parameters should start be [++port=number].", | ||
| vim.log.levels.WARN | ||
| ) | ||
| end | ||
| lp.pick(port) | ||
| else | ||
| lp.help() | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use uppercase letters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the lua JIT, there are no constant types. Thus naming convention must be used to indicate when something is a constant. In this case the prefix is a constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lua 5.1 doesn't have any convention for constant, and there is no need to invent one. Not to say that using uppercase letter for local constant is a bad idea, because you won't get LuaLS diagnostic if you mistype it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using capital letters for constants is a well-established convention in many programming languages (including C, C++, Rust, Python, Java, and JavaScript) to make them immediately distinguishable from variables whose values may change.
This improves readability and makes the intent explicit, especially in collaborative codebases. It’s not inventing a new convention, it’s applying a long standing one.
If i want to i can probably find examples in lua codebases as well.
While Lua doesn’t have an officially standardized convention for constants, borrowing this approach is not a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LuaCAT doesn't have constant annotation, but maybe this hack works
Haven't tested though