Skip to content

Conversation

mauricioszabo
Copy link
Contributor

No description provided.

savetheclocktower and others added 30 commits December 1, 2024 13:42
…for spellchecking on a per-language basis.

This opens up things that aren't really possible with the `excludedScopes` setting. For instance: you can specify `source.python comment, source.python string, source.js comment`; this will enable spellchecking in Python files for both comments and strings, while enabling spellchecking in JavaScript files for _only_ comments.

Closes #1135.
This PR also makes this compatibility check testable, and even expandable, by being able to provide a new platform if preferred.
* Add comments to `WASMTreeSitterLanguageMode`…

…and emit an event in `IndentResolver::suggestedIndentForBufferRow` that was missed last time.

* [language-c] Fix highlighting of `&` in a reference declarator

(Should've made it into last month’s!)

* [language-c] Move `reference_declarator` declaration to C++-only file…

…because it's not valid C.

* Attempt to fix chronically failing spec

I _think_ this is happening because we're not waiting for the watcher to start. `nsfw` goes async when the watcher is created _and_ when it's started.
…rm-compatibility-check

[settings-view] Fix Package keymap compatibility check
…nularity

[spell-check] Allow the user to whitelist sections of a buffer…
Need to fix a compilation error in CI which is
resulting from missing X11-related headers.

Some of these deps presumably have been removed or changed in the recent
"ubuntu-latest means Ubuntu 24.04" change.

See this tracking issue about the runner migration for details, maybe:
github[dot]com/actions[slash]runner-images/issues/10636

So, adding all of these deps just to be sure our code can compile
and be tested successfully in CI. These are the same deps as used
above in the main "build" Linux part of the workflow file.
…st-and-upload-Linux-job

CI: Add build dependencies for Linux 'test bins' job
Also, adjust the timeout duration to be shorter,
and adjust the number of retries to be greater.

I figure successful runs usually succeed quickly (~27 minutes or less?)
So, get to the retrying faster! But also, let's actually retry,
a bunch more times if we have to!
…ethod

[core] Expose `dbPromise` in `StateStore`
* [spell-check] Change test so it works identically across platforms

* [spell-check] Remove errant `console.log`…

…and clean up specs.

* [spell-check] Revert addition to `spell-check.grammars`…

…of the `"source comment"` value.

A user can add it back in if they want spell-checking of code comments, but it's probably a bit too opinionated to be the default.
…ls (#1185)

* [pulsar-updater] Don't prompt to update on non-default release channels

* [pulsar-updater] Fix spec
37 minutes is a bit long. 31 minutes should allow most runs that are
*going* to finish to finish, without wasting more time than we need to
waiting for runs that are hung and are never going to finish anyway.

Considered 27, since *most* runs finish at 16 minutes, and all but one
of my ~11 test runs were under 27 minutes, but *one* of my test runs was
just under 31 minutes, so this should do the trick.

Having to wait for an entire additional run by being over-eager to retry
(too impatient) feels wasteful and not the best trade-off.

Anyhow, this is closer to the right number, even if picking a better
"exact ideal timeout" may be possible, this is close enough.
Seems unreliable and not the right tool when we have apt-get.

Also, our "package tests" CI recently stopped working with an error
about broken/unsatisfiable package dependencies. This fixes it.

---

For those who are curious...

Passing (older image) details:

  Image: ubuntu-20.04
  Version: 20241215.1.0
  Included Software: https://github.com/actions/runner-images/blob/ubuntu20/20241215.1/images/ubuntu/Ubuntu2004-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu20%2F20241215.1

Vs failing (newer image) details:

  Image: ubuntu-20.04
  Version: 20250105.1.0
  Included Software: https://github.com/actions/runner-images/blob/ubuntu20/20250105.1/images/ubuntu/Ubuntu2004-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu20%2F20250105.1

(But now the tests pass even on the newer runner image, as of this fix.)
Attempt to install dependencies on new Ubuntu CI images
DeeDeeG and others added 27 commits March 5, 2025 22:21
* Remove run-tests scripts

Pro:
* Less code to maintain & update
* run-package-tests was unused

Con:
* Lost the formatting

* Drop support for JUnit XML output for tests

Pro:
* Less code to maintain

* Introduce helper/jasmine-singleton to have reference only to the vendored jasmine

* only the vendored version got used during the tests
* don't need to require jasmine multiple times

Clean up vendored jasmine to make it easier to upgrade

Remove checks about global jasmine as we're not setting it anymore

* Move jasmine-test-runner to be under spec/runners

* Introduce jasmine2 test runner

* Improve test runner switcher

* Fix spec files running with jasmine 2

* Upgrade archive-view to use jasmine2 test runner

* Add retry mechanism to jasmine2 test runner

* Write a wrapper to eliminate done callbacks from async tests

* Run flaky spec only on linux

* Fix the non-headless spec runner

Co-authored-by: Andrew Dupont <[email protected]>

* Fix core specs after merging from `master`

* Add ability to specify trailing comment pattern in `runGrammarTests`

* Add list reporter for CI

---------

Co-authored-by: Andrew Dupont <[email protected]>
* Upgrade to `web-tree-sitter` v0.25.3

This is a big upgrade! `web-tree-sitter` has been rewritten in TypeScript and has made some backwards-incompatible changes to its exports — all of which are good things.

I've also updated the instructions for generating the `web-tree-sitter` bundle.

* [language-python] Various folds fixes…

* Make function parameters spanning multiple lines fold properly
* Make `try`/`except` clauses fold properly, with `try` folding only up to the first `except`
* Add `folding-spec.js` to test these cases and others and guard them against regressions

Also fix a silly mistake in an indentation query.

Fixes #1216.

* [language-python] Convert specs to Jasmine 2 runner

* Get grammar specs working on Jasmine 2 runner

* Monkey-patch `Query::captures` more directly

* [language-ruby] Properly highlight and indent new `case/in` syntax

* [language-ruby] Properly fold `case` statements and `when/if` clauses

* Add logging to compare/contrast CI output between Pulsar and PulsarNext

* [language-javascript] Ensure `foo` in `this.#foo = 0` highlights properly

* Purge a stale fold boundaries iterator…

…instead of silently failing.

For some reason, a fold cache wasn't updating in PHP in certain circumstances. We were trying to resolve a fold on a given row by inspecting a capture whose node was useless to inspect because its tree had long since been invalidated.

This was only reproducible on startup, so maybe it has to do with non-root language layers not having their fold caches purged after tokenization?

This isn't easy to reproduce in a way that allows us to write a spec, so I've written this code very conservatively so that it can't possibly regress — only do slightly more work than it used to.

One way around this would be to extract the metadata we need when we create the boundaries iterator (so that we don't have to inspect the node later). That would theoretically increase the lifetime of the cache, but would involve doing more work up front. So it may or may not be worth it performance-wise. (It's not like the folds query is particularly costly, and it might be pointless to try to extend the cache's lifespan when code changes constantly and the _underlying data_ will be stale after a short while.)

* [language-php] Fold multiline comments

* Remove temporary logging
Includes the ppm Pull Request:
- Update to read v3 (ppm PR 150)
ppm: Update ppm to commit a6f843f0381f64cb5865efc7
….127.0

Changelog: Update changelog for Pulsar 1.127.0
meta: Update editor version to 1.127.0 (Pulsar v1.127.0 Release)
Copy link
Contributor

@savetheclocktower savetheclocktower left a comment

Choose a reason for hiding this comment

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

When you said you were going to make this PR a while back, I was a bit puzzled — I didn't think I had diverged from master in any meaningful way.

In fact, there are a couple places where I failed to pick up one CI change or another — but other than that, if I accepted this PR, it would undo a lot of work I've done to ensure specs pass and to get some important features working.

I think this PR could be cleaned up, but honestly I think the only legitimate changes worth picking up are in .cirrus.yml and build.yml, and even on those I'll defer to @DeeDeeG to decide if they're worth adopting.

So that should give you peace of mind — if you see any divergence from master in the main source code at this point, it's on purpose. Other than what's happened since 1.127.0, the updated-latest-electron branch is up to date with master, and tomorrow I'll bring in the changes since then and put out a PulsarNext 1.128.0.

APPLEID: ENCRYPTED[549ce052bd5666dba5245f4180bf93b74ed206fe5e6e7c8f67a8596d3767c1f682b84e347b326ac318c62a07c8844a57]
APPLEID_PASSWORD: ENCRYPTED[774c3307fd3b62660ecf5beb8537a24498c76e8d90d7f28e5bc816742fd8954a34ffed13f9aa2d1faf66ce08b4496e6f]
TEAM_ID: ENCRYPTED[11f3fedfbaf4aff1859bf6c105f0437ace23d84f5420a2c1cea884fbfa43b115b7834a463516d50cb276d4c4d9128b49]
ROLLING_UPLOAD_TOKEN: ENCRYPTED[41b279482c1b61c9129e410233cc5cd71bbfd60e33d03cefae451aff6bc9915d1706a6e8fefd0e0f9041a4bd287c84be] # <-- This is the Electron Next Bins token! Careful when resolving merge conflicts to not overrite to/from master. master branch keeps its token, updated-latest-electron branch keeps this one!
Copy link
Contributor

Choose a reason for hiding this comment

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

@mauricioszabo, can you revert the change on this line? Per the comment, this upload token should remain distinct on the updated-latest-electron branch.

describe('TextEditorComponent', () => {
beforeEach(() => {
if (!window.customElements.get('text-editor-component-test-element')) {
if(!window.customElements.get('text-editor-component-test-element')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No! These if(s shall not stand! :)

Comment on lines -3600 to -3608

// Set `line-height` to an integer to save us headaches later. (We
// believe that Chromium used to guarantee fixed increments of line
// height, but doesn't anymore — so we must specify it as an integer
// value ourselves if we want the math to be simpler.)
element.style.lineHeight = '20px';
component.measureCharacterDimensions();

// Make the editor tall enough to fit only two lines at a time.
Copy link
Contributor

Choose a reason for hiding this comment

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

All the additions in this file are purposeful; they're what I had to add to get specs to pass in Electron 30. They need to stay.

const fs = require('fs-plus');
const temp = require('temp').track();

const { conditionPromise: waitForCondition } = require('./helpers/async-spec-helpers');
Copy link
Contributor

Choose a reason for hiding this comment

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

All the changes in this spec file are on purpose and should stay.

const path = require('path');
const temp = require('temp').track();

const { conditionPromise } = require('./helpers/async-spec-helpers');
Copy link
Contributor

Choose a reason for hiding this comment

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

All the changes in this spec file are on purpose and should stay.

let pane4 = null;
let editor;

await atom.workspace
Copy link
Contributor

Choose a reason for hiding this comment

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

All the changes in this spec file are on purpose and should stay.

const AtomWindow = require('./atom-window');
const ApplicationMenu = require('./application-menu');
const AtomProtocolHandler = require('./atom-protocol-handler');
const { onDidChangeScrollbarStyle, getScrollbarStyle } = require('./scrollbar-style');
Copy link
Contributor

Choose a reason for hiding this comment

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

All the changes in this file are purposeful and should stay.

await parserInitPromise;
if (!this._language) {
try {
this._language = await WASMTreeSitterGrammar.loadLanguage(this.treeSitterGrammarPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a purposeful change and should stay.

"license": "MIT",
"atomTestRunner": "runners/jasmine2-test-runner",
"dependencies": {
"tree-sitter-python": "0.19.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's please not add back a Tree-sitter dependency.

@DeeDeeG
Copy link
Member

DeeDeeG commented May 13, 2025

Seems like a "process of how we work with these branches" PR as opposed to a strictly code-focused PR.

@savetheclocktower has been cherry-picking from master, this merges from master.

I dunno if it is necessary from a "syncing the code" perspective, but it might clarify the relationship of the branches from a revision history/version tracking (lower-level git-isms) perspective? Effectively marks how much diff we want to yeet vs master and what we want to confirm is desired, and let git understand such explicitly...

But the real thing is whether to be doing merges or cherry-picks, I think that should be down to the preferences of active contributors to this branch IMO. If there's, say, @savetheclocktower (preferring cherry-picks?) and @mauricioszabo (preferring merges?) that's a tie and I dunno.


But maybe a chat on the Discord could clear up how to handle history divergence (cherry-picks or merge commits) in the future for this work ??

(I don't think it makes sense to scrutinize this PR's diff closely without resolving that larger-picture question.)

@savetheclocktower
Copy link
Contributor

Maybe this will put everyone's mind at ease: updated-latest-electron is never going to get PR’d against master.

When we're ready (or nearly ready) to ship Pulsar on Electron 30, I'll create a new branch off of master. I'll use the diff between updated-latest-electron and master as a reference and a target to reach, but I'll recapitulate the changes in a much more logical series of commits. (One commit for removing legacy Tree-sitter, one for updating pathwatcher, etc.) When that candidate branch has zero diff with updated-latest-electron, it'll be suitable for landing.

In the meantime, rest assured I'm keeping this branch up to date.

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