validate-namespace-deletion webhook is called 2 times

How to categorize this issue?

/kind question
/priority normal
What happened:
validate-namespace-deletion webhook is called 2 times for single ns delete request.
What you expected to happen:
How to reproduce it (as minimally and precisely as possible):

Try to delete a project ns

$ k delete ns $PROJECT_NS

Ensure that the validate-namespace-deletion webhook is called 2 times:

time="2020-06-08T18:10:08+03:00" level=info msg="Rejected 'DELETE namespace' request of user 'minikube-user': Direct deletion of namespace \"<PROJECT_NS>\" is not permitted (you must delete the corresponding project \"<PROJECT>\")."
time="2020-06-08T18:10:08+03:00" level=info msg="Rejected 'DELETE namespace' request of user 'minikube-user': Direct deletion of namespace \"<PROJECT_NS>\" is not permitted (you must delete the corresponding project \"<PROJECT>\")."

Anything else we need to know?:
Environment:

Gardener version: master
Kubernetes version (use kubectl version):
Cloud provider or hardware configuration:
Others:

Ref https://github.com/kubernetes/kubernetes/issues/91917#issuecomment-640939654

/close

I also observed this behaviour for other admission plugins (also for UPDATE calls).
For example, if the ShootValidator denies the request (for example disallows finalizer removal), then it will be immediately invoked again.
It took a quick look and this seems to be done on purpose from the apiserver library, but I am not sure, what is the actual reason for it and if we can/should avoid or reconfigure it.

I also observed this behaviour for other admission plugins (also for UPDATE calls).
For example, if the ShootValidator denies the request (for example disallows finalizer removal), then it will be immediately invoked again.
It took a quick look and this seems to be done on purpose from the apiserver library, but I am not sure, what is the actual reason for it and if we can/should avoid or reconfigure it.

Generally I observed the same with out ShootValidators and started the investigation about them. Then I saw the same happening for validate-namespace-deletion webhook and spend effort on reproducing the issue with it.


Shadow DOM v1

Wondering how much of the v1 SDOM spec is supported here.
https://github.com/polymer/polymer/tree/2.0-preview#scoped-styling
https://developers.google.com/web/fundamentals/getting-started/primers/shadowdom

None at the moment, but v1 is being implemented alongside v0 – Opera and Chrome both support createShadowRoot and attachShadow, but Firefox is only createShadowRoot.
I will move across soon, but don't worry too much about it at the moment.

Will use attachShadow if it's available.


Syncthing support

Description
Syncthing support would be nice.
So that you can see the folders and their status in the dashboard.
Priority
Low (Nice-to-have)

I agree with you, would love to have it. Can you give me more info on what / how / how you would want it to look ? It would help me a lot.
Especially if you can provide a mock design or help me with the link to the API

I come more from the admin than the developer or designer area, so I can't contribute that much.
But I'll try to put a few things together here.
The normal overview looks like this:
For the dashboard, however, I think the folder overview is better/more suitable for now:

Other
I can generally adjust three widgets:
Overview
Shows more or less the same as the Overview of Syncthing itself. Download and upload speed etc.
Folder view
Shows the status of the folders in a simple list.
Speedmeter
Similar to the one for qBittorent, the time course of upload and download is displayed.
Useful links

https://docs.syncthing.net/dev/rest.html
Github Search: language:TypeScript syncthing

How about instead we make it like the categories, you can click on a folder to expand it ?
also we could add some progress bar if it is currently syncing or something

Expanding and progress bars sound very good, but you'll have to do that with the categeories.
For me, these are currently just grouping boxes, or have I missed something?

Would also love this feature, and could maybe contribute.
@ajnart do you have a guide somewhere or maybe a template PR on adding a new integration? I didn't have time to dig through the code or anything yet

Would also love this feature, and could maybe contribute.
@ajnart do you have a guide somewhere or maybe a template PR on adding a new integration? I didn't have time to dig through the code or anything yet

Hi,
thanks for your interest.
We do have guides here: https://homarr.dev/docs/community/developer-guides
Regarding the backend code, I'd suggest that you have a look at our existing widgets.
We use trpc for our API.
Let me know if you have any questions.
I'm happy to review a pull request regarding this feature.
Our core team sadly does not have the ability and capacity to implement each and every feature - therefore we'd appreciate your contribution.

Hey @ajnart  @manuel-rw,
if no one is working on this, I can start and submit PR once its ready. We can may be go into specifics later like UI, once I have functionality.

Hey @ajnart  @manuel-rw,
if no one is working on this, I can start and submit PR once its ready. We can may be go into specifics later like UI, once I have functionality.

Hi, that would be wonderful! However, the team is currently doing a complete overhaul of homarr for a v1.0 in a new repository. Feel free to join the discord to talk about it, it should be easier to implement new features in that version.

Hey @ajnart  @manuel-rw,
if no one is working on this, I can start and submit PR once its ready. We can may be go into specifics later like UI, once I have functionality.

Sure!
Let us know if you need help. Check homarr
dev for developer docs

@ajnart  I have joined discord. I am assuming this v1.0 repo is private until the release? If not can I contribute there?

@ajnart  I have joined discord. I am assuming this v1.0 repo is private until the release? If not can I contribute there?

You can, just check out the insiders channel to see to how to get access, the repo is homarr-labs/homarr


[Notion] Could we have the support for Notion's newest status property?

Extension – Notion
Author: @ice-Yaya

Describe the feature and the current behavior/state.
Loving 2 current functions to support Kanban view and customising visible properties for when searching for a database.
But the new status property is not an option in setting up the Kanban and choosing what to see.
Hope this could be supported.
Who will benefit with this feature?
As Notion launched this new status property feature, I believe more and more user will transform their initial single-selection into status. This will more relevant to the Kanban view in this Raycast extension.
Any Other info.

Notion help center on status property
Notion API info on status property

Notion has updated the API (reference)
More supports for the status property.

Now that it's possible, it would be nice yeah. Do you want to hack on it?


Random thought: two composefs output formats

@cgwalters mentioned something in a meeting today that I wasn't properly thinking about before.
Long story short: by doing the selinux label rewriting which we need to do as installing an unsealed/modified container image, we're effectively already creating a non-normative form of the container image that is only used for booting the system, and not from inside of containers running on the system.
If we're doing that anyway, then to this non-normative boot-only composefs we could make another tweak: remove /boot from the image before we create it, replacing it with an empty directory.  That works around our UKI hash recursion issue without getting into weird layer hiding tricks.
Of course, if we're talking about sealed images, then probably we already precomputed our selinux labels already and wrote them into the tar stream and then wrote the "one true" composefs fsverity digest into a label on the image.  In that case our hands are substantially more tied.  But maybe this idea of splitting out /boot makes sense anyway.  We already need to handle /boot resources specially on install, and in the UKI case we have a UEFI signature on the kernel binary (which, in turn, is also signed as part of the overall container image) which becomes the real trust chain for the booted system anyway (ie: we don't check the OCI label on boot).  So maybe having a composefs with /boot stripped from it still makes sense even for fully-sealed images.

The obvious advantage of this: we could stop thinking about all of these artifact/hidden-composefs-meta-layer hacks.  The boot resources would just be in /boot in BLS type 1 or type 2.

If we're doing that anyway, then to this non-normative boot-only composefs we could make another tweak: remove /boot from the image before we create it, replacing it with an empty directory. That works around our UKI hash recursion issue without getting into weird layer hiding tricks.

I am not quite following; we need to "physically" ship the kernel in the container image so it's downloaded to the client system right?
Isn't this issue just overall a duplicate of https://github.com/containers/composefs-rs/issues/21 ?

This issue is definitely highly related to #21 but sort of a different direction.  I mostly filed it because I wanted to write it down as I was flying out the door so I didn't forget :)


add NMT master self-start support

This adds support for bit 2 of 0x1F80 (NMT start-up) in accordance with CiA DSP 302 (v3.2.1) §5.1
If bit0 and bit2 of 0x1F80 are set then CO_NMT_process will check OD_errorRegister for active errors.  Once all error bits have been cleared by CO_errorReset the application will transition to NMT Operational.

Please verify split-driver branch, readme.md.
This part is changed and clarified there.

Sorry, I checked again, it is on master branch too.
But we have to discuss this functionality. Slave has only two allowed values for 0x1F80, as described in README.md, self start chapter. This is from DSP302-2(v4.1.0) standard.
I prefer putting this functionality into custom application code, if needed.

That works for me, thanks for the response!


About ref_stats in 'evaluator_v1.py'

In 'evaluator_v1.py' line 49 to 52, there refers to 'ref_stats_test.pkl' and 'ref_stats_spatial_test.pkl'. However the Google drive only provides 'imagenet_ref_stats.pkl'. Could you offer the download of these two pkl files? Thanks a lot.

Thanks a lot!


Add: Ramgarhi ABC

Channel Name
Ramgarhi ABC
Native Name (optional)
No response
Network (optional)
No response
Country
CA
Subdivision (optional)
CA-BC
City (optional)
Surrey
Broadcast Area
s/CA-BC
Languages
eng;pan
Categories (optional)
religion
NSFW
FALSE
Launched (optional)
No response
Closed (optional)
No response
Replaced By (optional)
No response
Website (optional)
https://ramgarhiabc.com/
Logo
http://ramgarhiabc.com/wp-content/uploads/2014/02/headerlogo.png
Notes
Streaming URL:
https://443-1.autopo.st/100/weblive/bcgurduwarabrookside/chunks.m3u8

https://iptv-org.github.io/?q=Ramgarhi ABC


Improve track_file handling and enable vrrp_track_file for keepalived < 2.1.0

Pull Request (PR) description
keepalived < 2.1.0 used (uses, it's e.g. in Ubuntu 20.04) "vrrp_track_file" instead of "track_file".
The PR uses a pragmatic approach to enabling this by simply cloning the type & code. Tested @ client, works as expected.
Also, the current implementation sets a hardcoded "weight" (to 1, which is the default anyway), but the "track_file" (and now "vrrp_track_file") has that as a parameter, so we should honour that.
And since the parameter is called "@param track_file" but actually wants an array, adds that to the description.
This Pull Request (PR) fixes the following issues
N/A

Linking to #278 and #281

@bluthg can you please rebase against our latest master branch to get rid of the merge commits?

@bluthg can you please rebase against our latest master branch to get rid of the merge commits?

Uh oh, me and git...

And now the deletion of .puppet-lint.rc is in the MR again... sigh!


GH Action tests for backend

Adds GH action to run backend tests and fixes some things along the way.

Maybe for the tests we only build 1 theme by passing it as an argument to the script?
https://github.com/OriginProtocol/dshop/blob/master/shop/buildThemes.sh#L31


Hold cmd & Q to quit on MacOS

just like chrome

Only to prevent users misoperation caused close the editor.
Then close the issue please.

Since VS Code has hot-exit I do not see a strong need for this...


Release v0.2.2

preparing for release of v0.2.2

fix internal dependency version mismatch
add missing tags to repo (v0.2.0, v0.2.1)
add missing changelog entries (v0.2.1, v0.2.2)

the lerna workflow is weird, create a branch + pull request and then publish prior to merge into main...

@pana-cc this is to catch up with the release cycle.

testdeck 0.2.2 has been released https://www.npmjs.com/settings/testdeck/packages

maybe we should consider to use a preview release prior to the preview, e.g.
npm run lerna-publish -- --preid

where the preid defaults to alpha.
this will publish 0.2.2-alpha packages instead of the final 0.2.2 release packages.
of course we can also choose to use preview instead of alpha.


starter site text and basic content

Callum, basic attempt at getting something up. If you haven't made any changes then feel free to use this or not. I'm going to point the domain at it so I have a reference point up.

The background image on the 'hero-banner-w-image' is for styling the background rather than for use as an actual picture. I.e. if you change the width of your browser so that it is thin, the actual picture (here the fitbit) will be placed under the text, but the background image goes behind the text and makes it unreadable.
I readded the old background, and I'll continue to make a few changes. I think it's also better to wait a day or so for new pictures of the app, because I'm trying to make it look slightly more presentable. (As a placeholder it's fine)

Yes I realized after I pushed. Esp noticeable on mobile form factor....
Needs fixing
On Sun, 29 Mar 2020, 03:12 Callum<EMAIL_ADDRESS>wrote:

The background image on the 'hero-banner-w-image' is for styling the
background rather than for use as an actual picture. I.e. if you change the
width of your browser so that it is thin, the actual picture (here the
fitbit) will be placed under the text, but the background image goes behind
the text and makes it unreadable.
I readded the old background, and I'll continue to make a few changes. I
think it's also better to wait a day or so for new pictures of the app,
because I'm trying to make it look slightly more presentable. (As a
placeholder it's fine)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/callumstew/ubuild-jekyll/pull/1#issuecomment-605547051,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAS72VMTFN4ULQ27VLWZ5P3RJ2U7LANCNFSM4LVTLFPQ
.


Accept-Encoding: gzip using Adapter/Curl always throws an error.

the HttpClient tries to decode the gzip response from the Curl response, but curl does that already.
Thus throws an exception.
$httpClient = new \Zend\Http\Client();
$httpClient->setOptions(array(
  'adapter' => '\Zend\Http\Client\Adapter\Curl',
  'timeout' => 5,
  'curloptions' => array(
    CURLOPT_TIMEOUT => 30,
    CURLOPT_IPRESOLVE => CURL_IPRESOLVE_V4,
    CURLOPT_ENCODING => 'gzip',
  ),
));
$httpClient->setUri('http://www.deep-freeze.ca/');
$httpClient->send();
// This throws an exception: "Error occured during gzip inflation"
echo $httpClient->getResponse()->getBody();

This issue has been closed as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


PR with benchmark results

Also use 0 explicitly for the number of torchvision threads.

Yeah, unfortunately that's the case. We can do a special case in a different PR


Implemented get_feature_names() for OneHot and Hashing transformers.

In reply to https://github.com/scikit-learn-contrib/categorical-encoding/issues/79.
I have implemented get_feature_names for OneHotEncoder and HashingEncoder as an example of how it could be done. Happy to get feedback before continuing on the remaining encoders.
I use both personally and find them to work well.

Too many issues. Will continue working on this.

Don't give up. There are just 2 issues that appear repeatedly:

When you train OneHotEncoder with drop_invariant=False, generated_cols is not defined.
n_components in HashingEncoder is not iterable.

Yeah, I won't, don't worry. Just need a bit more time for a thorough revamp. Will PR again soon ;)


Questions: Hollow Buttons ?

Why are Hollow buttons not like the usual ones?
for eg.

https://codepen.io/ncerminara/pen/qdeVGP/
https://laracasts.com/ homepage

Any specific reason ?

Why do you mean by "not like the usual ones"? The hover behavior?

Yup the hover behaviour

There are different behaviors for hollow buttons (the one in your examples is just one of them) and I don't think there is one which is better than the others.
This can be easily customized to meet your needs if you want to reproduce your examples' behavior.

but there should be some custom sass variables for that ...... to customize these type of things within the framework

We will close this since there is a PR for this #9740


[Roadmap] Contribute Thai version contract with inventor

https://github.com/kamranahmedse/developer-roadmap/blob/master/license

Inventor approved for using source:

tpa #20


🌱 Bump dependencies to support k8s v1.27

Issue: https://github.com/rancher/aks-operator/issues/269

Note: This is ready for reviews, but let's not merge this manually for now until we make sure Rancher also bumps these dependencies.


Nodes is not iterable (told in the server)

(node:3904) UnhandledPromiseRejectionWarning: TypeError: nodes is not iterable
at new Manager (C:\Users\jnsho\node_modules\lavacord\dist\lib\Manager.js:17:28)
nodes =
[
{
name: 'Node Test #1',
host: 'localhost',
port: 2333,
password: 'youshallpass'
}
]

(documentation was wrong) told me to do:
new manager(client, nodes, {})
turns out it was
new manager(nodes, client, {})

there is no client so both are wrong
its new Manager(nodes, options)
so new Manager(nodes, { user: "bot user id", send: () => { // do stuff here }; }); which i have explained in discord


Artifacts with +sub-stat <10 are not properly scanned

EDIT: Not just ATK, +DEF<10 also won't log, maybe Elemental Mastery as well, although I'd have to check 2star artifacts for that one.
Current Behavior
Artifacts with sub-stat ATK+ <10 are not properly scanned. Specifically the artifacts seem to be scanned but the sub-stat does not log. I have no idea if it's just this sub-stat or the 3star artifacts or anything of that sort. I can't pin it down, especially since I'm quite new to the game. The reason I'm saying <10 is because I found a similar (closed) issue that was reported about a year ago.
Expected Behavior
The scanner to correctly exports the data
Reproducing a Behavior
I'm pretty sure it's the <10 ATK thing, which is also what the other person found in the past.
Device OS
Windows 10
Genshin Impact Version
4.3, 4.4
Inventory Kamera Version
1.3.12
Screen resolution, screen mode, and UI scale
1920x1080 fullscreen, 1600x900 windowed
Additional notes and remarks
No response
Evidence

Preflight Checklist

[X] I have checked for similar issues in the issue page, and I have not found any similar to mine.
[X] I am using the latest version of the scanner.
[X] I tried setting the scanning delay to maximum, and it still doesn't work.
[ ] I have attached the video recording to the bug report.
[X] I have attached the error log to the bug report.
[X] I am available to later explains the issue.
[X] I have disabled all screen filtering software or plugins (i.e. HDR, Image Sharpening) while scanning.
[X] I use default in-game keybindings or have configured keybindings for the scanner.

Just downloaded the version 13.3.3 on Genshin Impact 4.4 and the problem seems to have been fixed. However, I'm now also having the issue with the rounded decimals (there is already an open issue about this).

This is fixed in the latest release


Store PrimaryKey sstable and row id information for fast comparison

This implementation uses a wrapper to store a PK's row source information to simplify comparisons.
After this first pass, I think we might benefit more if we embed this information into the PrimaryKey itself. Then, we could remove the explicit merge logic and hide it within the compareTo method in such a way that any comparison of PKs that results in equal PKs also results in merged source information. This could be tricky though in cases where the PK is only a token.

Marking as ready for review to get tests to run.

Alas, this is about 30% slower.
Before:
Cassandra_2023_11_28_182645.zip
After:
Cassandra_2023_11_28_182304.zip

ISTM that PrimaryKeyWithSource is missing the fast path branch of "if key1 only has a single source sstable, and key2 also has that source, then compare rowIds instead of the PK"

That makes sense to me, I'll push up a commit soon. With this change, the optimization gets better as compaction reduces PKs or for workloads that do not update rows. The JFR indicated checking for set intersection was expensive.

@jbellis - my most recent commit looks pretty valuable. In my testing, it looks like it went from 72% of the getNextRowId to 54%, leaving the remainder to the call to advance through the postingList. It's possible that using

ISTM that PrimaryKeyWithSource is missing the fast path branch of "if key1 only has a single source sstable, and key2 also has that source, then compare rowIds instead of the PK"

I interpreted this as an indication that we should only store a PK's single source sstable id and row id because collections require too much overhead. Based on your other comment, it sounds like I misinterpreted your comment.
I'll look at the collection operations again to see how we can improve it. For one, we spent a lot of time hashing objects. Given that the cardinality of sstables is pretty low, it's probably faster to just use a list and to iterate through it.

Latest is a modest improvement:
Cassandra_2023_11_29_110755.zip
There's still a lot of compareTo happening but we won't be able to eliminate that for intersections between different sources so maybe this is what victory looks like?

I'll look at the collection operations again to see how we can improve it.

My first impulse would be to reach for an IdentityHashMap<SSTableId, RowId>, fwiw.

I'm trying to understand where the win is coming from here.
Profiler says compareTo in RII.computeNext goes to 0.4% of parent from 7.1% of parent, which must be this line?
        while (highestKey != null && highestKey.compareTo(getMaximum()) <= 0)

The compareTo in RangeIterator.skipTo is slightly faster, the one in RangeUnionIterator is about 20% slower.
TTDR.ceiling is 30% faster which surprises me since I can't see any reason for it.  Is there just a ton of run-to-run variance?

        while (highestKey != null && highestKey.compareTo(getMaximum()) <= 0)

I took this line out (against vsearch HEAD) and it did not change the profile (i.e., compareTo still 7% of RII.computeNext).
That is puzzling b/c the only other direct use of compareTo is
                    if (nextKey == null || (comparisonResult = nextKey.compareTo(highestKey)) > 0)

but from the loop construction nextKey should never be from the same sstable as highestKey.

Ran the queries against both branches again.  This one is consistently faster than vsearch, most of the win is in

Significantly faster ceiling
Almost completely eliminating RII.computeNext direct invocations of compareTo
I don't understand either of those results.

Switching to review the JFR now

Why can't they be from the same sstable?

You're right, that was an incorrect assumption on my part.  That part makes sense now.

Significantly faster ceiling

Ah, this is because of your PLRI changes.  Ceiling didn't get faster, it's just called less often.

Right! This is the challenge with relative percentages. We need to get some query latency numbers soon.

Significantly faster ceiling

Ah, this is because of your PLRI changes. Ceiling didn't get faster, it's just called less often.

What did get faster should be getNextRowId

It feels like we could optimize this a lot more if we did as much as possible at the PL level, before switching to PK.
Like, if we know we've advanced the PL from SSTable A to row 23 and the PL from B to 75 in the first expression's RUI, then by the nature of intersections we can fast forward the PL/A and PL/B in the 2nd expression's RUI to 23/75 as well.  We only need to actually convert to PK when one of the expressions involves distinct sstables from the other.
Something to explore in another ticket?

It feels like we could optimize this a lot more if we did as much as possible at the PL level, before switching to PK.

In other words: we got a medium-sized win from skipping the ceiling computation when we get lucky and we happen to be in the same sstable as the one the highest key came from, but the more sstables there are the less significant this will be.  Can we force this to happen more often by breaking apart the abstraction layers?

Like, if we know we've advanced the PL from SSTable A to row 23 and the PL from B to 75 in the first expression's RUI, then by the nature of intersections we can fast forward the PL/A and PL/B in the 2nd expression's RUI to 23/75 as well. We only need to actually convert to PK when one of the expressions involves distinct sstables from the other.

Mmm, that doesn't quite work since knowing that A and B have been advanced to 23 and 75 doesn't actually tell you what the minimum values would be for the other expression.  You'd have to also know that A and B have SKIPPED OVER (say) 16 and 71.

we got a medium-sized win from skipping the ceiling computation when we get lucky and we happen to be in the same sstable as the one the highest key came from

This benefit might be a reason to look at merging PK sources and keeping track of the whole collection of sources.
Another thing that comes to mind is that we lazily do the performSkipTo logic. So it is ideal to make sure that we find the highest skipTo point before iterating.
Finally, it seems like the ceiling logic in PLRI is similar to the binary search in the limitToTopResults. Not sure if we could use a similar strategy to the limitToTopResults optimization, but it's possible we're doing a O(log(n)) search to skip only a few postings.

This benefit might be a reason to look at merging PK sources and keeping track of the whole collection of sources.

We only really need the sources when doing an intersection, and maybe when doing a range query since that also results in a call to skipTo the lower bound.
On the other hand, we could use these sources to skip the row id lookup from PK in the limitToTopResults method.

By using exactRowIdOrInvertedCeiling instead of ceiling, which has a huge optimization for PKs without clustering columns, this looks a lot better. It might be a reason to update the exactRowIdOrInvertedCeiling method so that we have an efficient search when clustering columns are present.

Latest is a modest improvement: Cassandra_2023_11_29_110755.zip

For reference, in the above JFR, ceiling is 89% of getNextRowId, but in my most recent one, exactRowIdOrInvertedCeiling is 12% of getNextRowId. I'm not exactly comparing apples to apples, but it still seems like a pretty huge win for the specific workload we're optimizing.

Great idea!  Confirmed that ceiling basically vanished as a hot spot.

Actually, let's refactor, then use ceiling

Cool, that's what I was going to suggest.

That leaves basically all of the profile as

compareTo in RangeUnionIterator.computeNext
compareTo in PLRI (via RUI invoking hasNext)
compareTo in RangeIterator.skipTo (invoked from RUI.performSkipTo)

In raw terms, I went from around 50k queries in 4.5 minutes to about 80k queries in the same time frame, which is about a 60% increase in throughput.

compareTo in RangeUnionIterator.computeNext

Does using a PQ like MergePostingList improve things when we have a bunch of sstables on disk?

Not sure, taking a look at that MPL#advance now

        if (state == State.READY && next.compareTo(nextToken) >= 0)
            return;

Taking this out makes it 150% faster, but also makes tests fail.  Frustratingly none of the iterator tests fail, but the higher level Vector tests do.

Does using a PQ like MergePostingList improve things when we have a bunch of sstables on disk?

RUI is essentially an implementation of a PQ, but it doesn't do any reordering and it does do de-duping.

The ArrayList operations took up about 0.8% of my last profile. They were pretty trivial to remove, so I did. It's still possible that we would benefit from an algorithm that doesn't iterate through all of the ranges every time though.

It's still possible that we would benefit from an algorithm that doesn't iterate through all of the ranges every time though.

That's why I suggested a PQ to get log(N) comparisons instead of N

Makes sense, I should have realized that, but I missed it. Looking at ways to update it now.

I put something together, but it doesn't work when skipTo is called...
    public PrimaryKey computeNext()
    {
        if (pq.isEmpty())
            return endOfData();
        var nextIteterator = pq.poll();
        var nextKey = nextIteterator.next();
        while (!pq.isEmpty() && pq.peek().peek().compareTo(nextKey) == 0)
        {
            var matchingIterator = pq.poll();
            matchingIterator.next();
            if (matchingIterator.hasNext())
                pq.add(matchingIterator);
            else
                FileUtils.closeQuietly(matchingIterator);
        }
        if (nextIteterator.hasNext())
            pq.add(nextIteterator);
        else
            FileUtils.closeQuietly(nextIteterator);
        return nextKey;
    }

It seems like we need a priority queue that has built in deduplication and also that allows us to call hasNext on RangeIterators before trying to order them.

The main difference is that a RangeIterator throws if you call peek when it is exhausted. Therefore, we'd need to add hasNext to the performSkipTo method after we call skipTo on the iterator. It's not the same pessimization you left a comment on, but it is suboptimal in the case where we have two calls to performSkipTo in a row without any calls to computeNext.

That's not to say it won't work, just to say there are some nuances that require special treatment. I'll go for the most straight forward implementation and see where that puts us.

Could we redefine peek to return null when exhausted?

Taking this out makes it 150% faster, but also makes tests fail.

Obviously (in retrospect) the reason it's failing is that everything is an AbstractIterator, so once you call hasNext on RUI, it "pulls a value out" of the sub-iterators into its own next field.  Without some really messy undo logic, it really is invalid to skip checking the value of next before proceeding to skipTo.
So, we'd like to divorce hasNext() from next() for efficient skipTo, but we also want to combine them because we want peek() functionality as well.  (and peek is used in several dozen places.)
I don't see an easy way to satisfy both requirements.

I don't see an easy way to satisfy both requirements.

I tried pushing it down into the implementations (so RUI does the compareTo with next, other iterators can either perform the compare or do the skip directly if they have a way to do so, the main thing we care about is PLRI which can avoid the compare) and this turns out to be a prettymassive pessimization for the case we have here where there is a huge amount of overlap between the predicates, primaryKeyMap.ceiling(skipToToken) is something like 2x slower than compareTo so we end up losing.
Pushed here in case I missed something obvious but this might be a dead end. https://github.com/datastax/cassandra/tree/skipto-without-compareto

I think PKWithSource gets obsoleted by compare-by-token, so let's leave that on the shelf for now.  I pulled the other changes here into https://github.com/datastax/cassandra/pull/888


Configure raises an error if CTest scripts print to stdout

Brief Issue Summary
My project uses TEST_INCLUDE_FILES to generate labels for CTest tests at runtime.
Our script prints a line Generating test labels... to stdout. This causes CMake Tools to report an error when configuring.
[rollbar] Unhandled exception: Unhandled Promise rejection: cleanConfigure SyntaxError: Unexpected token G in JSON at position 0 {}

[error] SyntaxError: Unexpected token G in JSON at position 0
	at JSON.parse (<anonymous>)
	at CTestDriver.refreshTests (/root/.vscode-server/extensions/ms-vscode.cmake-tools-1.17.17/dist/main.js:50861:37)
	at async /root/.vscode-server/extensions/ms-vscode.cmake-tools-1.17.17/dist/main.js:44894:29
	at async CMakeProject.doConfigure (/root/.vscode-server/extensions/ms-vscode.cmake-tools-1.17.17/dist/main.js:45001:24) cmake.cleanConfigure {"value":"ms-vscode.cmake-tools","_lower":"ms-vscode.cmake-tools"}

This can easily be reproduced.
cmake_minimum_required(VERSION 3.27)
project(foo)

enable_testing()

file(CONFIGURE
    OUTPUT ${PROJECT_BINARY_DIR}/createTestLabels.cmake
    CONTENT "execute_process(COMMAND @CMAKE_COMMAND@ -E echo \"Generating test labels...\")\n"
    @ONLY
)

set_property(DIRECTORY APPEND PROPERTY
    TEST_INCLUDE_FILES ${PROJECT_BINARY_DIR}/createTestLabels.cmake
)

{
    "version": 6,
    "configurePresets": [
        {
            "name": "gcc-debug",
            "binaryDir": "${sourceDir}/out/build/${presetName}",
            "cacheVariables": {
                "CMAKE_CXX_COMPILER": "g++",
                "CMAKE_BUILD_TYPE": "Debug"
            },
            "generator": "Unix Makefiles"
        }
    ],
    "buildPresets": [
        {
            "name": "gcc-debug",
            "configurePreset": "gcc-debug"
        }
    ],
    "testPresets": [
        {
            "name": "gcc-debug",
            "configurePreset": "gcc-debug"
        }
    ]
}

CMake Tools Diagnostics
No response
Debug Log
No response
Additional Information
No response

@benschreiber Thank you for your github issue! We followed the command you provided to reproduce the issue, but we encountered error below, in order to better reproduce and solve your issue, can you provide us with your reproduction steps video and reproduction file?

It looks like you are on windows? Then you can modify the preset to use MSVC and the VS Generator. But it would be easier to just try to reproduce on Linux or in WSL.

@Yingzi1234 If you're still stuck, try using this devcontainer.json
{
    "image": "mcr.microsoft.com/devcontainers/cpp:debian-12",
    "customizations": {
        "vscode": {
            "extensions": [
                "ms-vscode.cpptools",
                "ms-vscode.cmake-tools"
            ]
        }
    }
}

@benschreiber Thanks for your reply, based on the latest information you provided, we tried to reproduce your issue on a Linux machine, but are not sure if we got the same results as your issue. When I run the "CMake: configure" command, an error box pops up, but when I go to the output window, the contents show that my configuration was successful. Here's a recording of my reproduction for you to check out.
Uploading GitHub issue.webm…

We verified the issue on CMake tools: v1.19.45 and it has been fixed.
ENV：
CMake Tools:v1.19.45
vscode: 1.92.2
Actual result:


RFC: remove restrictions on baggage item keys

Per the resurrection of the (dormant) https://github.com/opentracing/opentracing.github.io/issues/25#issuecomment-220827152 , I propose that we get rid of the restrictions on baggage item keys (per SetBaggageItem/GetBaggageItem). Those restrictions were put in place because (a) HTTP is an important and popular inter-process transport, (b) it is appealing to encode individual baggage items as individual HTTP headers, yet (c) HTTP header keys have weird restrictions due to numerous undocumented assumptions and transformations made my numerous distinct HTTP libraries and tools.
Though there's something to be said for the logic above, it feels like the cart is leading the horse. HTTP is not the only transport, nor is it necessary to make the literal baggage item key a component of a corresponding HTTP header key.
TL;DR: The proposal is to simply remove all restrictions on the keys (beyond that they be valid unicode strings).
cc some people who've had opinions about this in the past or have dealt with OT implementation: @adriancole @yurishkuro @dkuebric @lookfwd @michaelsembwever @bg451 @bcronin @jmacd @cattail

the impact of this is case preservation becomes an implementation detail
and so folks will have to code accordingly (ex with knowledge that the keys
might not come out the same at the other end).
It might mean http tracers become either more opaque on the wire (being no
longer able to map across headers in order to preserve case (via values)),
or deviant (libraries choosing not to honor the case-sensitivity
constraint).
Regardless, this topic has been well discussed, so nothing more to add.
Thanks for the ping

To me, the main drawback is that it complicates the baggage serialization logic in the tracers, especially if we want to keep them backwards compatible with already deployed tracers. But it definitely makes the API easier to use for end users, so I am ok with removing restrictions on baggage keys.

:+1:

(I'm making some progress on this during the SpanContext change, but we can't really close this until https://github.com/opentracing/opentracing.github.io/issues/98 is done)

This is subsumed by https://github.com/opentracing/opentracing.github.io/issues/98 (and the SetBaggageItem comments no longer mention these restrictions in any platform).


Can someone help me with this error?

After I type "python EnigmaCracker.py" this is what it says:
Downloads\EnigmaCracker-main\EnigmaCracker.py': [Errno 2] No such file or directory

try
python ./EnigmaCracker.py
or
python ~/EnigmaCracker.py
or go by terminal into the directory and force it


textDocument/definition implementation for VS Code.

Trial of implementation of textDocument/definition request. Supports types only, and only in case when type is found in Zinc analysis. When source(s) is found then editor opens potential source(s). This simple implementation don't use semantic data (yet).

[x] move vscDefinition task to separate file
[x] add tests

still in progress, no worries about that. I have to clean it first.
2017-10-24 6:34 GMT+02:00 eugene yokota<EMAIL_ADDRESS>
@eed3si9n requested changes on this pull request.
In main/src/main/scala/sbt/Defaults.scala
https://github.com/sbt/sbt/pull/3660#discussion_r146451167:

@@ -1387,6 +1390,91 @@ object Defaults extends BuildCommon {
}
analysisResult.analysis
}

lazy val previousAnalysis = Def.inputKey[String]("gets previous analysis")
lazy val previousAnalysisParser = Def.setting {
Parsers.StringBasic
}
lazy val previousAnalysisCommand = Command.single("previousAnalysis") { (state, definition) =>

Why are we adding both an input key and a command here?
In main/src/main/scala/sbt/Defaults.scala
https://github.com/sbt/sbt/pull/3660#discussion_r146451248:

   }

 }

val commandAnalysis = state.get(Keys.analysis)
state.log.warn(s"command analysis: $commandAnalysis")
val previousCompile = Project.runTask(Keys.previousCompile in Compile, updatedState).collect {

 case (newstate @ _, Value(v)) if v.analysis.isPresent => v.analysis.get

}
state.log.warn(s"previous analysis: $previousCompile")
val cmdSrc = state.currentCommand.flatMap(_.source)
import sbt.internal.langserver.{ Location, Position, Range }
val params = Location(input.textDocument.uri, Range(Position(0, 0), Position(0, 10)))
val cname = cmdSrc.map(_.channelName).get
val chnl = StandardMain.exchange.channels.collectFirst {

 case c if c.name == cname => c

}
chnl.foreach(_.publishEvent(params, responseId))

If possible, we should use event logging here.
In main/src/main/scala/sbt/Defaults.scala
https://github.com/sbt/sbt/pull/3660#discussion_r146451437:

   }

   .toSeq

 state.log.warn(s"tasks $tasks")

 val complete = Aggregation.timedRun(updatedState, tasks, sbt.std.Transform.DummyTaskMap(Nil))

 state.log.warn("here")

 val r = complete.results.toEither.toOption.map { results =>

   state.log.warn("and there")

   results.map(_.value)

 }

 r

}.getOrElse(Seq.empty)
state.log.warn(s"aggregated analysis $aggrAnal")

val analysis = {

 val compile = Keys.compile in Compile

 Project.runTask(compile, updatedState, true).collect {

Is this invoking compilation task just to get to Analysis?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#pullrequestreview-71393894, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSzO9nk9CZ0Nx7FiY0AUZRWniGiN7ks5svWjugaJpZM4QB0Hy
.

could anybody tell me what's wrong with tests? Locally they are passing here not.

Hi @wpopielarski! I tried to test this branch, but couldn't make it work.

There is one thing that I had to change locally: to comment these two lines
https://github.com/sbt/sbt/blob/9232a8fbc872c2f94a91ec82d797a11140a271a0/main/src/main/scala/sbt/Defaults.scala#L1412-L1413
This is called in the compileIncremental task, that I guess, is used in your lspDefinition in some way. The result is that servers bombards client with empty publishDiagnostics notifications (to clean up), but this happens in response to every textDocument/definition request. On the sbt codebase it means that every request takes at least 20 seconds. And even on a small project it's ~5s.
Commenting these lines out is not a solution, but the point is, that textDocument/definition shouldn't trigger diagnostics cleanup at all
On a sidenote, I think that diagnostics cleanup should happen somewhere else. An alternative approach is to store diagnostics and every time new ones are generated, "merge" them and send only the necessary changes, this way there is no need to send lots of empty diagnostics on every compile. This is just an idea and, of course, out of scope of this PR.

After that I could see textDocument/definition requests/responses. And I couldn't get any results: responses are always empty. I tried on the sbt code, on some work project, on a tiny test project — it didn't work anywhere. Could you provide an example code on which it is supposed to work well?

By the way, I tested it with Atom, but I think that there is nothing VSCode-specific happening here, so it sould work the same in both editors. I should install VSCode and try it out too...

No luck in VS Code either:

I have a feeling that I'm holding it from the wrong end.. 😅

Just Foo is not class, trait, object so not available in zinc analysis
file. Right now jump to definition is made from these analysis files not
scalameta-sbt plugin.
2017-11-05 5:44 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
No luck in VS Code either:
[image: screen shot 2017-11-05 at 05 39 16]
https://user-images.githubusercontent.com/766656/32412064-46f1254c-c1ec-11e7-8e64-fdc80943789d.png
I have a feeling that I'm holding it from the wrong end.. 😅
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341948887, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS9iYfaz6e_pMAofAq5_ViyLb-4PKks5szT1KgaJpZM4QB0Hy
.

Alexey,
there are not my lines and I think you should ask Eugen Yokota about the
reason of these 2 lines. Actually it is hard to say why you get empty
response. One of the reason can be the lack of zinc analysis cache files
which are used to find products of compilation. So probably you have to
compile project first and than try to jump to definition. And because type
identifiers are got from these files so if you are asking about identifier
which is not found there (even if it is scala Int, String or any other
common type) you get 'No definition found'.
2017-11-05 5:25 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Hi @wpopielarski https://github.com/wpopielarski! I tried to test this
branch, but couldn't make it work.

There is one thing that I had to change locally: to comment these two
lines
https://github.com/sbt/sbt/blob/9232a8fbc872c2f94a91ec82d797a1
1140a271a0/main/src/main/scala/sbt/Defaults.scala#L1412-L1413
https://github.com/sbt/sbt/blob/9232a8fbc872c2f94a91ec82d797a11140a271a0/main/src/main/scala/sbt/Defaults.scala#L1412-L1413
This is called in the compileIncremental task, that I guess, is used
in your lspDefinition in some way. The result is that servers bombards
client with empty publishDiagnostics notifications (to clean up),
but this happens in response to every textDocument/definition
request. On the sbt codebase it means that every request takes at least 20
seconds. And even on a small project it's ~5s.
Commenting these lines out is not a solution, but the point is, that
textDocument/definition shouldn't trigger diagnostics cleanup at all
On a sidenote, I think that diagnostics cleanup should happen
somewhere else. An alternative approach is to store diagnostics and every
time new ones are generated, "merge" them and send only the necessary
changes, this way there is no need to send lots of empty diagnostics on
every compile. This is just an idea and, of course, out of scope of this PR.
2.
After that I could see textDocument/definition requests/responses. And
I couldn't get any results: responses are always empty. I tried on
the sbt code, on some work project, on a tiny test project — it didn't work
anywhere. Could you provide an example code on which it is supposed to work
well?
By the way, I tested it with Atom, but I think that there is nothing
VSCode-specific happening here, so it sould work the same in both editors.
I should install VSCode and try it out too...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341948327, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS2pFqzTNw0p62upb8CX84OqTz1Hvks5szTjLgaJpZM4QB0Hy
.

Just Foo is not class, trait, object so not available in zinc analysis file.

One of the reason can be the lack of zinc analysis cache files
which are used to find products of compilation. So probably you have to
compile project first and than try to jump to definition.

 Tried with defining a class, a trait, an object — nothing works..
As I said before, could you provide some example code which works for you?

there are not my lines and I think you should ask Eugen Yokota about the reason of these 2 lines.

Sure, I wrote that more for @eed3si9n

Right now jump to definition is made from these analysis files not scalameta-sbt plugin.

I was actually thinking about it recently and was going to ask you. It seems that using information from the scalameta's semantic DB would be a straightforward way to get any symbol information for the language server. But I guess it's out of sbt's scope, because it requires a third-party plugin..
Anyway, in this light, is it still worth digging zinc analysis cache to get only partial information only for some symbols? Or would it make more sense to just implement this functionality outside of sbt, using scalameta-sbt (and interact with it through sbt server)?

Really hard to say. It's working for me. So my question is: do you get
compilation errors? This part should work as well. You should get also info
as logs about received requests by sbt server
2017-11-05 17:32 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Just Foo is not class, trait, object so not available in zinc analysis
file.
One of the reason can be the lack of zinc analysis cache files
which are used to find products of compilation. So probably you have to
compile project first and than try to jump to definition.
[image: screen shot 2017-11-05 at 17 17 33]
https://user-images.githubusercontent.com/766656/32416681-6ef3d4c0-c24d-11e7-8ee2-1b925e20684c.png
I definitely compiled the project and I don't get definitions on any
symbol in this little piece of code. Tried with defining a class, a
trait, an object — nothing works..
As I said before, could you provide some example code which works for you?
there are not my lines and I think you should ask Eugen Yokota about the
reason of these 2 lines.
Sure, I wrote that more for @eed3si9n https://github.com/eed3si9n
Right now jump to definition is made from these analysis files not
scalameta-sbt plugin.
I was actually thinking about it recently and was going to ask you. It
seems that using information from the scalameta's semantic DB would be a
straightforward way to get any symbol information for the language server.
But I guess it's out of sbt's scope, because it requires a third-party
plugin..
Anyway, in this light, is it still worth digging zinc analysis cache to
get only partial information only for some symbols? Or would it make more
sense to just implement this functionality outside of sbt, using
scalameta-sbt (and interact with it through sbt server)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341985989, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS1AHIamWirF9gIfKVQoqwuotplucks5szeMegaJpZM4QB0Hy
.

So my question is: do you get
compilation errors? This part should work as well. You should get also info
as logs about received requests by sbt server

It compiles well, no errors (really simple code on the screenshot above). And I do see logs about receiving requests. I also see empty responses in Atom (can I see client-server communication log somewhere in VS Code?).

about scalameta-sbt. Finding a symbol helps to find its definition by
determining its type or some local stable identifier or whatever else but
it still need work to figure out its source file and position in this file.
So analysis files are just first step in this way I guess
2017-11-05 18:00 GMT+01:00 Wieslaw Popielarski<EMAIL_ADDRESS>
Really hard to say. It's working for me. So my question is: do you get
compilation errors? This part should work as well. You should get also info
as logs about received requests by sbt server
2017-11-05 17:32 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Just Foo is not class, trait, object so not available in zinc analysis
file.
One of the reason can be the lack of zinc analysis cache files
which are used to find products of compilation. So probably you have to
compile project first and than try to jump to definition.
[image: screen shot 2017-11-05 at 17 17 33]
https://user-images.githubusercontent.com/766656/32416681-6ef3d4c0-c24d-11e7-8ee2-1b925e20684c.png
I definitely compiled the project and I don't get definitions on any
symbol in this little piece of code. Tried with defining a class, a
trait, an object — nothing works..
As I said before, could you provide some example code which works for you?
there are not my lines and I think you should ask Eugen Yokota about the
reason of these 2 lines.
Sure, I wrote that more for @eed3si9n https://github.com/eed3si9n
Right now jump to definition is made from these analysis files not
scalameta-sbt plugin.
I was actually thinking about it recently and was going to ask you. It
seems that using information from the scalameta's semantic DB would be a
straightforward way to get any symbol information for the language server.
But I guess it's out of sbt's scope, because it requires a third-party
plugin..
Anyway, in this light, is it still worth digging zinc analysis cache to
get only partial information only for some symbols? Or would it make more
sense to just implement this functionality outside of sbt, using
scalameta-sbt (and interact with it through sbt server)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341985989, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS1AHIamWirF9gIfKVQoqwuotplucks5szeMegaJpZM4QB0Hy
.

I don't mean 'correct code' :) I mean if you make failing snippet do you
get error compilation back?
2017-11-05 18:05 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
So my question is: do you get
compilation errors? This part should work as well. You should get also info
as logs about received requests by sbt server
It compiles well, no errors (really simple code on the screenshot above).
And I do see logs about receiving requests. I also see empty responses in
Atom (can I see client-server communication log somewhere in VS Code?).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341988324, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS6M1SeAPIeiS4uwlqDfIk9unFDX_ks5szerbgaJpZM4QB0Hy
.

I don't mean 'correct code' :) I mean if you make failing snippet do you get error compilation back?

😄 I see. Yes, this part works fine, errors are reported as diagnostics.

Finding a symbol helps to find its definition by determining its type or some local stable identifier or whatever else but it still need work to figure out its source file and position in this file. So analysis files are just first step in this way I guess

You mean that even with scalameta we will need analysis cache from zinc to map symbols to the source locations? I thought it's also stored in the SemanticDB, but now I see that it's not.

but what you can see in log from:
https://github.com/sbt/sbt/pull/3660/files#diff-82bb5e5687c35f3190dd111f8a9d93bbR166
2017-11-05 18:18 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
I don't mean 'correct code' :) I mean if you make failing snippet do you
get error compilation back?
😄 I see. Yes, this part works fine, errors are reported as diagnostics.
Finding a symbol helps to find its definition by determining its type or
some local stable identifier or whatever else but it still need work to
figure out its source file and position in this file. So analysis files are
just first step in this way I guess
You mean that even with scalameta we will need analysis cache from zinc to
map symbols to the source locations? I thought it's also stored in the
SemanticDB, but now I see that it's not.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341989270, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSwYGM5YTVMR0iqI9uK8F929c2Hj6ks5sze4DgaJpZM4QB0Hy
.

Oh, I didn't see these logs. Should they appear in the sbt shell? If I don't get them, what am I doing wrong?

even this one?
https://github.com/sbt/sbt/pull/3660/files#diff-82bb5e5687c35f3190dd111f8a9d93bbR134
2017-11-05 18:39 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Oh, I didn't see these logs. Should they appear in the sbt shell? If I
don't get them, what am I doing wrong?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341990799, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSy9Uq_cDp4_99JrVVPDo6O8Dxiy6ks5szfLXgaJpZM4QB0Hy
.

so as I got you, you checked out vsc-definition branch and published it
locally and in your test project you set sbt version to this locally
published and it failed?
2017-11-05 18:43 GMT+01:00 Wieslaw Popielarski<EMAIL_ADDRESS>
even this one? https://github.com/sbt/sbt/pull/3660/files#diff-
82bb5e5687c35f3190dd111f8a9d93bbR134
2017-11-05 18:39 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Oh, I didn't see these logs. Should they appear in the sbt shell? If I
don't get them, what am I doing wrong?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341990799, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSy9Uq_cDp4_99JrVVPDo6O8Dxiy6ks5szfLXgaJpZM4QB0Hy
.

Nope, I only see (in the console)
[debug] onRequestMessage: JsonRpcRequestMessage(2.0, 8, textDocument/definition, {"textDocument":{"uri":".../test.scala"},"position":{"line":13,"character":17}}})

And then empty textDocument/publishDiagnostics notifications. I also see that Atom receives an empty response:
Scala (sbt) rpc.sendRequest textDocument/definition sending Object ...
...
Scala (sbt) rpc.sendRequest textDocument/definition received (5673ms) []

But no logs from lspDefinitionTask anywhere.

Yes, I checked your branch, published locally and I run test project with that locally published version. And I know that it's that version because I see the changes and I see logs about definition requests.

can you ran task 'lspDefinition` in sbt console?
2017-11-05 18:51 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Yes, I checked your branch, published locally and I run test project with
that locally published version. And I know that it's that version because I
see the changes and I see logs about definition requests.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341991685, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSwiA5JePeuZ2yBQQ64sM1N-ti89Bks5szfWrgaJpZM4QB0Hy
.

Yes, but I don't know what arguments to give it. I tried with the raw JSON request, but it didn't work:
 lspDefinition """{ "jsonrpc": "2.0", "method": "textDocument/definition", "params": {"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}}} }"""
[warn] Incorrect definition request Some({ "jsonrpc": "2.0", "method": "textDocument/definition", "params": {"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}}} })
[success] Total time: 0 s, completed Nov 5, 2017 6:54:38 PM

ok, first 'lspDefinition {"textDocument":{"uri":"file:///.../test.
scala"},"position":{"line":15,"character":13}' but it looks like this task
is not visible, i don't know... I take a look tomorrow at this
2017-11-05 19:08 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Yes, but I don't know what arguments to give it. I tried with the raw JSON
request, but it didn't work:
lspDefinition """{ "jsonrpc": "2.0", "method": "textDocument/definition", "params": {"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}}} }"""
[warn] Incorrect definition request Some({ "jsonrpc": "2.0", "method": "textDocument/definition", "params": {"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}}} })
[success] Total time: 0 s, completed Nov 5, 2017 6:54:38 PM
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341992807, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS_oXYzy5qZzCievOLMeUXem7zqdqks5szfmfgaJpZM4QB0Hy
.

OK, I figured out that it was about log level, I changed it to warn in the code and now I see the logs:
> lspDefinition {"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}}
[warn] lsp-definition raw request: List({"textDocument":{"uri":"file:///.../test.scala"},"position":{"line":15,"character":13}})
[warn] lsp-definition found line:   def qux(f: Foo): String = f.b
[debug] langNotify: JsonRpcNotificationMessage(2.0, textDocument/publishDiagnostics, {"uri":"file:/.../test.scala","diagnostics":[]})
[warn] lsp-definition potentials: Set()
[warn] lsp-definition potentials: Set()
[success] Total time: 1 s, completed Nov 5, 2017 7:27:56 PM

but Foo is type Foo or class Foo?

Class Foo just as you see on the screenshot from https://github.com/sbt/sbt/pull/3660#issuecomment-341985989, no other Foo in scope.

see at:
https://github.com/sbt/sbt/pull/3660/files#diff-82bb5e5687c35f3190dd111f8a9d93bbR61
I need some integration test too I see

Didn't notice your edit in https://github.com/sbt/sbt/pull/3660#issuecomment-341994983 at first.. 😄
I added a package, tried putting Foo inside the Buh object, nothing helps.. Here's previousCompile output:
> show previousCompile
[info] PreviousResult(analysis: Optional[Analysis: 3 Scala sources, 5 classes, 2 binary dependencies], setup: Optional[MiniSetup(output: SingleOutput(/Users/laughedelic/dev/laughedelic/scala-adder/target/scala-2.12/classes), options: MiniOptions(classpathHash: [Lxsbti.compile.FileHash;@68d57037, scalacOptions: [Ljava.lang.String;@5f0669d9, javacOptions: [Ljava.lang.String;@1c4f3b3d), compilerVersion: 2.12.4, order: Mixed, storeApis: true, extra: [Lxsbti.T2;@211d17c)])
[success] Total time: 0 s, completed Nov 5, 2017 8:04:14 PM

generally I tested it on the whole sbt project itself and working quite cool (I even did demo for @eed3si9n  and @sschaef ). Will see tomorrow then... Sorry for that.

good news is that is is working for simple projects too. But I have some
complex configuration taken from sbt project. I try to narrow it.
2017-11-05 20:06 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Didn't notice your edit in #3660 (comment)
https://github.com/sbt/sbt/pull/3660#issuecomment-341994983 at first..
😄
I added a package, tried putting Foo inside the Buh object, nothing
helps.. Here's previousCompile output:

show previousCompile
[info] PreviousResult(analysis: Optional[Analysis: 3 Scala sources, 5 classes, 2 binary dependencies], setup: Optional[MiniSetup(output: SingleOutput(/Users/laughedelic/dev/laughedelic/scala-adder/target/scala-2.12/classes), options: MiniOptions(classpathHash: [Lxsbti.compile.FileHash;@68d57037, scalacOptions: [Ljava.lang.String;@5f0669d9, javacOptions: [Ljava.lang.String;@1c4f3b3d), compilerVersion: 2.12.4, order: Mixed, storeApis: true, extra: [Lxsbti.T2;@211d17c)])
[success] Total time: 0 s, completed Nov 5, 2017 8:04:14 PM

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341996737, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSzSDjMWDfSGekatiNlfS6tUdx3Mfks5szgdGgaJpZM4QB0Hy
.

well, looks that extremely simple build.sbt must work. The only important
observation is that the very root project folder must be open by vs
code.Like this:

2017-11-06 12:18 GMT+01:00 Wieslaw Popielarski<EMAIL_ADDRESS>
good news is that is is working for simple projects too. But I have some
complex configuration taken from sbt project. I try to narrow it.
2017-11-05 20:06 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Didn't notice your edit in #3660 (comment)
https://github.com/sbt/sbt/pull/3660#issuecomment-341994983 at first..
😄
I added a package, tried putting Foo inside the Buh object, nothing
helps.. Here's previousCompile output:

show previousCompile
[info] PreviousResult(analysis: Optional[Analysis: 3 Scala sources, 5 classes, 2 binary dependencies], setup: Optional[MiniSetup(output: SingleOutput(/Users/laughedelic/dev/laughedelic/scala-adder/target/scala-2.12/classes), options: MiniOptions(classpathHash: [Lxsbti.compile.FileHash;@68d57037, scalacOptions: [Ljava.lang.String;@5f0669d9, javacOptions: [Ljava.lang.String;@1c4f3b3d), compilerVersion: 2.12.4, order: Mixed, storeApis: true, extra: [Lxsbti.T2;@211d17c)])
[success] Total time: 0 s, completed Nov 5, 2017 8:04:14 PM

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-341996737, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSzSDjMWDfSGekatiNlfS6tUdx3Mfks5szgdGgaJpZM4QB0Hy
.

and one remark: the selected definition is not found in the very first time for clean project (no compilation done). The second attempt is successful.
REMARK: I didn't test it without removed by you lines in Defaults

ok, I see big problems when these two lines are removed. What is interesting, I cannot drop back to previous behavior.

funny thing, I can see definition of Foo in internal.Foo.scala only

so I don't have any idea how previousCompile is working

🙁 it's confusing..
On a sidenote, guys from scalameta are actively working on the go-to-definition implementation right now: https://github.com/scalameta/language-server/pull/8

to calm down in 'previousCompile` usage. It's ok. I suffer with toolbox problem. testing...

@laughedelic could you please, get the commit up and check if definition is now working? There was my fault. Unfortunately ToolBox.parse is more powerful I expected. So now to check if given token is longest valid scala identifier I ask tb to compile this snippet of code.

Done 👌 It worked for me for the first time! But only with objects. Doesn't work with anything else..

really, not working for plain classes? or classes in objects?
2017-11-06 19:05 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Done 👌 It worked for me for the first time! But only with objects.
Doesn't work with anything else..
[image: screen shot 2017-11-06 at 19 04 34]
https://user-images.githubusercontent.com/766656/32456294-67ad8746-c325-11e7-9c6f-45de0f785654.png
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-342234125, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSxh19seOYooXlLXw_ETpuVf0g9v4ks5sz0p4gaJpZM4QB0Hy
.

Nope. On the screen shot above I have class Foo, tried putting it in or out of the object, doesn't work either way.

I'm just asking because right now is very simple (and naive) assumption
about simple class names. Only suffices .A, .A$, $A$ and $A are available.
2017-11-06 19:24 GMT+01:00 Wieslaw Popielarski<EMAIL_ADDRESS>
really, not working for plain classes? or classes in objects?
2017-11-06 19:05 GMT+01:00 Alexey Alekhin<EMAIL_ADDRESS>
Done 👌 It worked for me for the first time! But only with objects.
Doesn't work with anything else..
[image: screen shot 2017-11-06 at 19 04 34]
https://user-images.githubusercontent.com/766656/32456294-67ad8746-c325-11e7-9c6f-45de0f785654.png
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-342234125, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzSxh19seOYooXlLXw_ETpuVf0g9v4ks5sz0p4gaJpZM4QB0Hy
.

I don't know about classes.... Here you are:

@laughedelic could you play a while?

@wpopielarski I'll try it out tonight 👍

@wpopielarski Now it does work for objects, classes and traits 👍

so should be command used here? This part of code is using
previousCompile to collect analyses. Is any other way to figure out
analysis files names and locations?
2017-11-08 5:05 GMT+01:00 eugene yokota<EMAIL_ADDRESS>
@eed3si9n requested changes on this pull request.
In main/src/main/scala/sbt/internal/server/LanguageServerProtocol.scala
https://github.com/sbt/sbt/pull/3660#discussion_r149570600:

@@ -60,6 +58,12 @@ private[sbt] trait LanguageServerProtocol extends CommandChannel {
langRespond(InitializeResult(serverCapabilities), Option(request.id))
case "textDocument/didSave" =>
append(Exec("compile", Some(request.id), Some(CommandSource(name))))

 case "textDocument/definition" =>

   import sjsonnew.support.scalajson.unsafe.CompactPrinter

   append(

     Exec(s"lspDefinition ${CompactPrinter(json)}",

The query for definition, if possible, should happen concurrently with
whatever that's happening in the command processing engine. For example,
the user could be in the middle of test that takes 10 min. I think
textDocument/definition should return while the test is running, not
afterwards. So I don't think we should use task or input task here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#pullrequestreview-74985143, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS8noLoCFYOGrqAIe_E5uLMqm4Z3Nks5s0SiMgaJpZM4QB0Hy
.

I am guessing that you'd need to set up a cache of Analysis files yourself based on the known locations.

that's clear. thanks
2017-11-08 6:45 GMT+01:00 eugene yokota<EMAIL_ADDRESS>
I am guessing that you'd need to set up a cache of Analysis files yourself
based on the known locations.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sbt/sbt/pull/3660#issuecomment-342717057, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALEzS0gmkQV7NY8LIU4EYkegGrxuEUh6ks5s0T_0gaJpZM4QB0Hy
.

@laughedelic @sschaef  could you play with is a little please?

Hi @wpopielarski! I just tried out 9152ece. It's much more stable than before! It works on all objects, classes and traits in my test project 👍

Although I can't say that it's usable yet. There are several problems:

When I hover on empty space it responses with a lot of {code: -32700, message: "Symbol not found in definition request"} errors. I think it shouldn't send an error and ParseError is probably meant for the cases when you cannot parse the request itself. I also checked what other language servers do:

Flow server sends a response with null result (indicating success) and
and Python sends an empty response [] (no symbols found).
IMO Flow server does the right thing

It's unclear what to expect from this functionality, which things are supposed should work and which not. I think you should clarify it. You say

Supports types only, and only in case when type is found in Zinc analysis

but I have no idea what's found in Zinc analysis. Also, does it mean that it will never work for values/methods/etc.?

It's super slow 🐌  Some unsuccessful(!) requests take ~1.5s, and most successul ones take >1s. I see a spinning wheel on my mouse pointer 😅 I understand that this is work in progress and something could be optimized, but I think it's supposed to be several orders of magnitude faster..

Overall, I think that this is a very good experiment, but I really doubt now that it should be a part of the sbt server core. Taking into account that it's implemented through a task, it could probably be an sbt plugin? Or is it really tied to the sbt internals?
In my opinion sbt server development should be focused more on the interaction with the build, not sources analysis. There are already two (three if you count Dotty) language servers for Scala that provide this functionality and many other things: ENSIME and Scalameta (see https://github.com/scalameta/language-server/pull/23).
@wpopielarski I don't want to discourage you from improving this, I just want everybody working on things related to Scala and LSP be more aware of each other's work.

@laughedelic class trait and object are for good start. zinc can give info about methods and class fields too. I think we will use scalameta finally. meantime I add super efficient optimization (if working :) )

well, even with simple classes is problem because names are encoded so there is no other possibility than use scalameta :)

@dwijnand
looks like there is no dependency in compile. For simple maven project dependency:tree returns:
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ my-app ---
[INFO] com.mycompany.app:my-app:jar:1.0-SNAPSHOT
[INFO] \- com.github.ben-manes.caffeine:caffeine:jar:2.6.0:compile
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

the transitive dependencies of scalacache-caffeine look fine too:
com.github.cb372:scalacache-caffeine_2.12:0.20.0 [S]
+-com.github.ben-manes.caffeine:caffeine:2.5.6
+-com.github.cb372:scalacache-core_2.12:0.20.0 [S]
| +-org.scala-lang:scala-reflect:2.12.3 [S]
| +-org.slf4j:slf4j-api:1.7.25
|
+-org.slf4j:slf4j-api:1.7.25

I'm closing this in favor of https://github.com/sbt/sbt/pull/3777

If you could give me push rights, I could put those commits here too.

@eed3si9n thanks a lot for flattening history. I did the mess. And I think if https://github.com/sbt/sbt/pull/3777 is merged so is there a need to commit it to this branch? If I got you correctly.


Eror handling

Add error handling to the module(application throws exception when user inputs wrong data)

We have wrappers to handle errors on postRequest and getRequest functions, but there is no handling for class methods