Skip to content

Conversation

@niten94
Copy link
Contributor

@niten94 niten94 commented Feb 23, 2025

The repository where the file is located could be different with the current directory, so the functions in the status plugin are changed to run commands in the directory of the file.


function branch(b)
local function parseRevision(b, opt)
if b.Type.Kind ~= buffer.BTInfo then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One Addition:
In case we now compare against buffer.BTInfo shouldn't we use b.Type instead of b.Type.Kind?
Because the b.Type.Kind holds the int, but the b.Type the BufType.

Copy link
Contributor Author

@niten94 niten94 Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BufType.Kind is exposed to plugins with the same name as BufType constants, so b.Type.Kind has to be compared:

micro/cmd/micro/initlua.go

Lines 129 to 134 in 272a308

ulua.L.SetField(pkg, "BTDefault", luar.New(ulua.L, buffer.BTDefault.Kind))
ulua.L.SetField(pkg, "BTHelp", luar.New(ulua.L, buffer.BTHelp.Kind))
ulua.L.SetField(pkg, "BTLog", luar.New(ulua.L, buffer.BTLog.Kind))
ulua.L.SetField(pkg, "BTScratch", luar.New(ulua.L, buffer.BTScratch.Kind))
ulua.L.SetField(pkg, "BTRaw", luar.New(ulua.L, buffer.BTRaw.Kind))
ulua.L.SetField(pkg, "BTInfo", luar.New(ulua.L, buffer.BTInfo.Kind))

I do not know if the exposed values could somehow be changed without breaking API. Other exposed constant types are int but I was confused when I tried to compare with b.Type a year ago. Thinking about it now, I think a note could be added in documentation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Thank you for this hint. 👍
Not really intuitive, but yes...right now fix in this API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding buffer types, it would be beneficial to have access to the buffer kind parameter in the NewBuffer*() functions. Currently, you cannot directly create buffers that are not of type BTDefault, so even if you change the type, the onBufferOpen() function will always receive a BTDefault. As a result, you have to use alternative methods to differentiate between buffers.

micro/cmd/micro/initlua.go

Lines 135 to 140 in 272a308

ulua.L.SetField(pkg, "NewBuffer", luar.New(ulua.L, func(text, path string) *buffer.Buffer {
return buffer.NewBufferFromString(text, path, buffer.BTDefault)
}))
ulua.L.SetField(pkg, "NewBufferFromFile", luar.New(ulua.L, func(path string) (*buffer.Buffer, error) {
return buffer.NewBufferFromFile(path, buffer.BTDefault)
}))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't have enough free time at this part of the year even with other things.

Other exposed constant types are int but I was confused when I tried to compare with b.Type a year ago. Thinking about it now, I think a note could be added in documentation.

I plan to add a note about buffer.BT* constants and option registration (mentioned in sparques/micro-quoter#8 (comment)), but I am not good at writing documentation so someone else may still do it.

Currently, you cannot directly create buffers that are not of type BTDefault, so even if you change the type, the onBufferOpen() function will always receive a BTDefault.

Please create another issue. I haven't honestly read your comments in other issues much, but for now I suggest to avoid long text like what I did in the description of #3578.

@@ -1,9 +1,12 @@
VERSION = "1.0.0"
VERSION = "1.1.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version of builtin plugins does not seem to be updated and there is only a fix done in this pull request, so I don't know if this would be fine. I'll revert this sometime around this week.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version as such doesn't make a lot of sense for builtin plugins (they are not released separately from micro, not downloaded or installed separately from micro). So yes, I think it's better to keep this 1.0.0, just for consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about VERSION = "builtin", which results in this particular case in status (0.0.0-builtin)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better to leave it as-is unless it is causing problems. It is sort of user-facing so changes could potentially break something (https://xkcd.com/1172/).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By taking https://xkcd.com/1172/ serious we would end up in no change at all. 😉
The user is still able to override builtin plugins with local ones. Don't you think it would be nice to identify this use case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user is still able to override builtin plugins with local ones. Don't you think it would be nice to identify this use case?

I'm not sure how the version number is related, surely you can do that regardless of the version number? 🤔 Or do you just mean that the text "builtin" would then not be visible in the output of micro -plugin list (assuming the user remembered to change the version number in their override)? I don't really think that matters much.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or do you just mean that the text "builtin" would then not be visible in the output of micro -plugin list (assuming the user remembered to change the version number in their override)?

Yes, that's what I meant.
Assuming the user wouldn't touch a copied builtin version number leads to the situation that we can't differentiate it again.

So, ok...we keep it as is, unless anybody comes up with a better idea.

Return current commit hash and branch of repository where file in buffer
is located instead of current directory.
@niten94 niten94 force-pushed the status-pass-repodir branch from 7758da8 to 85e2b3b Compare March 2, 2025 01:57
@dmaluka dmaluka merged commit 2ae9812 into zyedidia:master Mar 4, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants