Skip to content
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

feat: Add general IDE info to User-Agent for Tabby clients #2781

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

Sma1lboy
Copy link
Collaborator

@Sma1lboy Sma1lboy commented Aug 3, 2024

This addresses the remaining part of the issue mentioned in #2581.

  • Retrieve client info based on the client configuration during client initialization.

Example User-Agent format:

Node.js/v20.9.0 Visual-Studio-Code-desktop/1.91.1 TabbyML.vscode-tabby/1.8.0-dev

@Sma1lboy
Copy link
Collaborator Author

Sma1lboy commented Aug 3, 2024

Should I do another PR for intellij? or I can put them together

This PR is waiting for #2766

@Sma1lboy Sma1lboy marked this pull request as ready for review August 3, 2024 11:11
@wsxiaoys wsxiaoys requested a review from icycodes August 5, 2024 00:17
@icycodes
Copy link
Member

icycodes commented Aug 5, 2024

@Sma1lboy Thanks for your PR!
As we are using the standard user-agent header, it would be better to follow this format, so that we can extend a standard user-agent parser to parse it on the server-side.
My suggestions:
Node.js/v18.18.2 (Linux 5.15.0-117-generic; linux x64) TabbyAgent/1.8.0-dev Visual-Studio-Code/1.92.0 TabbyML.vscode-tabby/1.8.2

  1. Node/Browser
    • For Node.js: Node.js/${nodejsVersion} (${osType} ${osRelease}; ${osPlatform} ${osArch})
    • For Browser, use value of: navigator.userAgent
  2. tabby-agent: TabbyAgent/${tabbyAgentVersion}, like TabbyAgent/1.8.0-dev
  3. IDE: ${ideName}/${ideVersion}, replace spaces with -, like Visual-Studio-Code/1.92.0
  4. Tabby IDE extension/plugin: ${pluginName}/${pluginVersion}, like TabbyML.vscode-tabby/1.8.2

@wsxiaoys for review.

Copy link
Member

@icycodes icycodes left a comment

Choose a reason for hiding this comment

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

About the implementation:

  1. We can get the ide info at LSP initialization, so it is not required to pass it in each completion call. Please refer to protocol definition and implementation passing params to TabbyAgent class. It's not required to modify code in vscode/intellij plugin, all stuff will be done in tabby-agent.
  2. Let's set the header user-agent at http client level.

@Sma1lboy
Copy link
Collaborator Author

Sma1lboy commented Aug 6, 2024

Sure, I'll take a look tonight

@Sma1lboy Sma1lboy marked this pull request as draft August 6, 2024 19:08
@Sma1lboy Sma1lboy force-pushed the feat-adding-ide-info-clients branch from 8c15f1d to 73e51d0 Compare August 7, 2024 08:31
@Sma1lboy
Copy link
Collaborator Author

Sma1lboy commented Aug 7, 2024

@icycodes Thanks for your suggestions. I've realized that most of my previous code was focused on creating a parameter for ideInfo that could be transmitted by the client. It wasn't until I read the section of the code about lifecycle that I discovered this process had already been written there. :(
I deleted the unnecessary code and cleaned up the user agent format. By the way, if I want to get the current version of the user agent, do I need to read the package.json? I haven't found any code that stores "packagejson" context.

@icycodes
Copy link
Member

icycodes commented Aug 7, 2024

if I want to get the current version of the user agent, do I need to read the package.json? I haven't found any code that stores "packagejson" context.

Simply import the "package.json" should be ok. Here is an example.

@Sma1lboy Sma1lboy marked this pull request as ready for review August 7, 2024 16:41
@Sma1lboy Sma1lboy requested a review from icycodes August 7, 2024 17:30
clients/tabby-agent/src/TabbyAgent.ts Outdated Show resolved Hide resolved
clients/tabby-agent/src/TabbyAgent.ts Outdated Show resolved Hide resolved
clients/tabby-agent/src/TabbyAgent.ts Outdated Show resolved Hide resolved
@Sma1lboy Sma1lboy requested a review from icycodes August 8, 2024 17:52
@Sma1lboy Sma1lboy requested a review from wsxiaoys August 9, 2024 05:58
@wsxiaoys wsxiaoys merged commit 4245c57 into TabbyML:main Aug 9, 2024
4 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.

3 participants