Skip to content

Conversation

@tartarughina
Copy link
Collaborator

When check_updates is set to "once", the extension now correctly checks for updates only once per component, preventing repeated GitHub API calls on every Zed restart.

Previously, the "once" mode would check GitHub API on every restart when no local installation existed, making it functionally identical to "always" mode until a download completed.

Changes:

  • Add persistence mechanism using .update_checked marker files
  • Store downloaded version in marker files for future reference
  • Implement has_checked_once() and mark_checked_once() helpers
  • Update should_use_local_or_download() to check marker before allowing download
  • Apply fix to all components: JDTLS, Lombok, Debugger, and JDK

Behavior after fix:

  • First run: GitHub API called → Download → Create marker with version
  • Subsequent runs: No GitHub API call (uses local or returns error)

Marker files created:

  • jdtls/.update_checked
  • lombok/.update_checked
  • debugger/.update_checked
  • jdk/.update_checked

When check_updates is set to "once", the extension now correctly checks
for updates only once per component, preventing repeated GitHub API
calls on every Zed restart.

Previously, the "once" mode would check GitHub API on every restart when
no local installation existed, making it functionally identical to
"always" mode until a download completed.

Changes:
- Add persistence mechanism using .update_checked marker files
- Store downloaded version in marker files for future reference
- Implement has_checked_once() and mark_checked_once() helpers
- Update should_use_local_or_download() to check marker before allowing
  download
- Apply fix to all components: JDTLS, Lombok, Debugger, and JDK

Behavior after fix:
- First run: GitHub API called → Download → Create marker with version
- Subsequent runs: No GitHub API call (uses local or returns error)

Marker files created:
- jdtls/.update_checked
- lombok/.update_checked
- debugger/.update_checked
- jdk/.update_checked
playdohface
playdohface previously approved these changes Dec 22, 2025
Copy link
Collaborator

@playdohface playdohface left a comment

Choose a reason for hiding this comment

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

Looks good and makes sense to me. Caveat: I haven't thoroughly tested that it works as expected on my setup, I could do that later tonight if you want

.filter_map(Result::ok)
.map(|entry| entry.path())
.filter(|path| path.is_dir())
.filter_map(|path| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I have the feeling that the filter_map - map - filter - filter_map can be better expressed as a single filter_map, something along the lines of:

std::fs::read_dir(&jdk_path)
    .ok()?
    .filter_map(|entry| {
        let entry = entry.ok()?;
        let metadata = entry.metadata().ok()?; // I think we even avoid the second syscall this way

        if !metadata.is_dir() {
            return None;
        }

        let time = metadata.created().ok()?;
        Some((entry.path(), time))
    })
    .max_by_key(|&(_, time)| time)
    .map(|(path, _)| path)
    ```

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The same filter_map - map - filter - filter_map sequence is used also in other places, like in jdtls.rs .
We could have it cleaned throughout the project in a different PR

@tartarughina
Copy link
Collaborator Author

Looks good and makes sense to me. Caveat: I haven't thoroughly tested that it works as expected on my setup, I could do that later tonight if you want

As I created a new revision right after your approval take the due time to test. There's no need to push it right away.

Copy link
Collaborator

@playdohface playdohface left a comment

Choose a reason for hiding this comment

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

Appears to work as intended for me 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants