Write a Bitmap Font Importer

The current way of importing Fonts into Duality is working, but quite limited and a bit buggy at times due to less-than-ideal Font handling in GDI+ / System.Drawing. A viable alternative would potentially improve usability and at the same time provide a sample on how to write custom Bitmap Font Importers.
Pick a commonplace bitmap font generator and implement a custom importer for it. Specifically, the AngelCode Bitmap Font Generator looks quite good.
This can be done easily in a distinct plugin and no special core code is necessary for this.

Hey @ilexp I'd love to try help out with this, and it sounds pretty simple, I'm just wondering how the actual importing would work from a user point of view - since there are two files involved, a BMFont file and the actual image. Is it assumed they are named the same and that a user would drag in the BMFont file?
Also are there any similar custom importers I could use as a guideline?

With the new Asset import / export API of v2.0, an import operation can involve multiple files and you can explicitly specify that N different files make up one single Resource.
Ideally, the user would dragdrop all involved files into the editor at once, but it is up to your importer implementation whether missing files are tracked down based on path assumptions such as being in the same directory and named all the same. You could also scan the BMFont file for filename cues in that case, should the format specify them.

Also are there any similar custom importers I could use as a guideline?

Unfortunately not - all existing importers have a 1:1 mapping between imported files and generated resources. However, I tried to supply all the required information via code documentation. As a general overview, you can take a look at the IAssetImporter and IAssetImportEnvironment interface definitions, as well as existing importer implementations and the extension methods they use to share and simplify code. You will likely not use the extension methods since they're targeting 1:1 mapping scenarios, but they're documented to help you understand what's going on.

@ilexp Is there any way to stop an import of a file - currently if I drag drop in both a .fnt and a .png file, I can create the font off the .fnt file, but the png is picked up by the SpriteImporter as well and is imported as a Pixmap.
Also, is export required, and in this case what would be expected?

@ilexp It's going great so far, but I'm struggling a little to align the characters to the baseline. The BMFont definition has a yoffset value per char but the Glyph class does not. How do you calculate the y position for each char? I also don't know what the difference is between Ascent and BodyAscent.
I understand that kernings are offsets to a character when it is placed next to another specific character but it does not seem like the current Font implementation uses it like this? I do not understand the fat that it's just a int[] on each side, but in some cases you check there are 6 segments? Could you explain this a little or push me in the tight direction to some information?
I have been reading and re-reading the Font class but I'm a little cross-eyed :wink:  by now...

@mika76 Nice work so far! :) I'll just quickly go over some of your questions:

Is there any way to stop an import of a file - currently if I drag drop in both a .fnt and a .png file, I can create the font off the .fnt file, but the png is picked up by the SpriteImporter as well and is imported as a Pixmap.

The key here is not to stop other import operations, but to tell the system your importer is going to handle the .png file as well. In the PrepareImport step, make sure to call env.HandleInput on all files you're going to use. That way, the import system will be able to detect the conflict and spawn a user dialog which importer to prefer. To modify default behavior here, you can also return a higher-than-zero value in the Priority property, since your importers use case (font AND image) is more specialized than the Pixmap import case (just an image).

Also, is export required, and in this case what would be expected?

No, export is not required. It serves two purposes:

Determining which source files belong to the imported Resource. This is used to decide what should happen when double-clicking the Resource, as well as to synchronize the Source/Media directory with moved or renamed files in Data.
Extracting previously imported data for editing.

You can safely omit export functionality entirely, it's an optional feature - though users might expect it to be there. Double-clicking a bitmap font could easily be a convenient "Okay, let me edit the image file of the font by hand now" - that's one thing the export would allow.

It's going great so far, but I'm struggling a little to align the characters to the baseline. The BMFont definition has a yoffset value per char but the Glyph class does not. How do you calculate the y position for each char?

Duality fonts use glyphs that have a constant height, so that offset needs to be encoded in the image. However, I do recognize that this is kind of a nonsense requirement that merely arises for historical reasons.
If you're ready to take the extra mile and dive into the Duality part of this, you can add an OffsetY field to GlyphData and implement the Y offset logic in ProcessTextAdv and all the methods using that one. As long as you don't modify any public API except for the added GlyphData field, this should not be a breaking change and could be released as a regular update. Feel free to do a Pull Request on this, should you go down that road.
The other alternative would be to simply adjust the pixel data after loading, but before passing it to the Font resource. This can be a part of the import process and does not require any core changes, thus you won't have to go through the core dev / Pull Request / review / approval / release cycle stuff.

I also don't know what the difference is between Ascent and BodyAscent.

See this Wikipedia image for reference:

While the Ascent value represents the "ascender height" from the image, BodyAscent refers to the "x-height", or height of the median line.

I understand that kernings are offsets to a character when it is placed next to another specific character but it does not seem like the current Font implementation uses it like this? I do not understand the fat that it's just a int[] on each side, but in some cases you check there are 6 segments? Could you explain this a little or push me in the tight direction to some information?

Duality doesn't use kerning pairs, mostly for historic reasons, since the default System.Drawing.Font based importer simply has no way of accessing this data and thus cannot provide it. Instead, fonts in Duality do kerning based on sampling how many transparent pixels there are at the side of each glyph using fixed "sample heights".
Each of the int[] indices represents one height, and the number in the array represents the number of transparent pixels from the respective side. All of this is calculated automatically and there's not really a way to use a different kerning algorithm right now.
This isn't ideal and will certainly change at some point, but unlike the OffsetY issue I wouldn't recommend you to tackle this in a PR since that part of code might be subject to a later (v3.0?) redesign and I'd like to give this some thought. So for now, I think you could just discard the provided kerning pairs data and hope for the best.
So.. that turned out to be a bigger answer than anticipated. Hope I could help you clear some things up!

Thanks @ilexp for the detailed answer :smile:
I went with the route of changing the Font class to allow for a offsetX field - works really well. Currently the importer works quite well with some limitations:

I am only parsing the xml version o BMFont. I'm not sure how much the text file version is required. XML has the same data all the programs I've tried so far export it, so not sure if the effort of writing the parser is warranted. Time will tell...
It seem the pixeldata class draws a rectangle when there is an atlas involved? Or is this just in the visulalizer? I'm finding if I dont include a padding when generating the font, I'm seeing some extra lines - I'm not sure if this is from the pixeldata or from the calculations used int the front placing, but that's tomorrow's problem :smile:
BMFont saves it's font data in specific channels (no colours or transparency), which I'm not dealing with currently (it was done this way I guess to allow for using the font data with a material or something). Currently they are displayed with the black background since I don't manipulate the texture currently.
BMFont format allows for multiple pages as attachments, but currently only a single page is supported. I'm not sure if I will support more since it means I would have to create a custom bitmap and re-calculate the glyps considering the Font class does not support it. Not sure it's worth the trouble.

Any comments on the above?

Ps: Do you have any helper classes I can use to extract a single channel like alpha onto it's own bitmap or something like that?

Pps: Knowing the code a little better now, I'm thinking I could add kerning to the Font class pretty easily - are you sure you'd like me to stay away from that? It seems they can be extracted from a font using PInvoke method http://stackoverflow.com/questions/16699862/how-do-you-use-windows-gdi-methods-in-net-that-arent-in-gdi so even the normal font generator could use them.

This is starting to look really nice. Thumbs up!

It seem the pixeldata class draws a rectangle when there is an atlas involved? Or is this just in the visulalizer? I'm finding if I dont include a padding when generating the font, I'm seeing some extra lines - I'm not sure if this is from the pixeldata or from the calculations used int the front placing, but that's tomorrow's problem :smile:

That's just the visualizer telling you there are atlas rectangles. I suspect the padding is required due to texture filtering, which is fine and kind of expected. The default font importer also uses padding, for that same reason. Disabling texture filtering on the other hand just doesn't look nice when scaling or rotating the font, so I think padding is a good solution here.

Ps: Do you have any helper classes I can use to extract a single channel like alpha onto it's own bitmap or something like that?

Nope, no helper classes for this. You'll have to iterate over the pixeldata to do this.

Pps: Knowing the code a little better now, I'm thinking I could add kerning to the Font class pretty easily - are you sure you'd like me to stay away from that? It seems they can be extracted from a font using PInvoke method http://stackoverflow.com/questions/16699862/how-do-you-use-windows-gdi-methods-in-net-that-arent-in-gdi so even the normal font generator could use them.

Introducing kerning pairs is a bit of a code design thing, which I'm always hesitant to assign to new contributors because it often takes a bit until someone really "gets into" the framework - and I really try to avoid handing out tasks and then having to reject Pull Requests people have been putting serious work into. That said, you seem to be on a roll here, so if you're up to it, I'd say do it. Just two things I'd suggest:

Let's talk about how you're going to implement this before you start.
Do a separate Pull Request for the OffsetY thing and the kerning pairs, so I don't have to decide to take both or none. This means, you'll need one branch for each of them, but since the offset change is much more basic, I'd say it's fine if the kerning branch builds up on the offset branch.

Oh, and thanks for the stack overflow link! I wasn't aware this was actually possible.

Any comments on the above?

Overall, I think it's a good thing you're starting with the basics and don't try to support every quirk and feature of the BMFont format right from the beginning. No need to go the extra mile for a feature if it turns out no one uses it.
Did you try the regular Duality kerning with BMFonts by the way? How did that hold up?

Introducing kerning pairs...

Ok I'll finish up with the basics here then see about this.

Overall, I think it's a good thing you're starting with the basics and don't try to support every quirk and feature of the BMFont format right from the beginning. No need to go the extra mile for a feature if it turns out no one uses it.

I agree. Also people were just not consistent. There are a couple of programs I have run into now which have totally different file formats.

Did you try the regular Duality kerning with BMFonts by the way? How did that hold up?

No not yet - I will play with it a bit more and send through a pull request before moving onto other stuff...

Allright, sounds good :+1:

@ilexp One question - in the editor, once I import a bitmap font, all the properties seem to be disabled. Do you maybe know why this is? I can't seem to trace it to anything specific.

@ilexp Found it - CanRenderFont is checking for an EmbeddedFont...

I think you're looking for this. It's in there because editing those properties only make sense if the editor can actually re-render the font with those new settings, which is currently determined by checking whether the font includes an embedded TrueType font. This hardcoded kind of relation will go away eventually, but for now we'll have to deal with it.
(I'm kind of sorry for the rabbit hole you've fallen into with this issue - did not expect you to run into that amount of historic code / hacks :smile: )
Is this a problem with regard to your importer?

:smile: It's not a problem - best way to learn something is to dive right in I think.
Also I fell into the trap of seeing the graphics render and thinking it's all working 100%, now I can see that changing the values is causing some issues I missed. One question - I can't think currently of any time you might have a Font without an EmbeddedFont (currently, without the bitmap stuff)? So I'm playing with changing the logic slightly for those checks in the Font class.  Also, soon I'll make a report so you can see what's been done. It probably sounds a lot bigger than it really is.

One question - I can't think currently of any time you might have a Font without an EmbeddedFont (currently, without the bitmap stuff)?

There's no need to keep it in there when releasing a game, even though the editor might be included in that release - so that's a case that should be supported, even though it doesn't happen in the "primary user story".
Also, as you noted, imported bitmap fonts - from your importer or others / custom ones - don't have an embedded truetype font, so there's that.

Also, soon I'll make a report so you can see what's been done. It probably sounds a lot bigger than it really is.

Nice, looking forward to it.

There's no need to keep it in there when releasing a game, even though the editor might be included in that release - so that's a case that should be supported, even though it doesn't happen in the "primary user story".

I thought by this you mean deleting the ttf file from the Media folder, but when I tried that, the EmbeddedFont still has it's full data (I tried with acustom, non-installed font). I'm assuming it's serialized? In which case how would it be cleared?
Basically I'm trying to have a check whether a font is a bitmap font or an embedded font, without adding a new property :smile: the question is is it safe to assume that if EmbeddedFont is null it means this would be a bitmap font.

I thought by this you mean deleting the ttf file from the Media folder, but when I tried that, the EmbeddedFont still has it's full data (I tried with acustom, non-installed font). I'm assuming it's serialized? In which case how would it be cleared?

It could be cleared either manually (deleting the property in the XML of the Font Resource) or as part of an automated deployment step (which nulls the property and re-saves the Font Resource). The Media/Source folder is not required or referred to by a Resource in any way, since they follow the overall goal of being self-contained within the Duality content system.

Basically I'm trying to have a check whether a font is a bitmap font or an embedded font, without adding a new property :smile: the question is is it safe to assume that if EmbeddedFont is null it means this would be a bitmap font.

In fact, all Duality Fonts are bitmap fonts! Some just happen to carry a .ttf around so the editor can re-render them when a user requests to do so. This feature, however, will be moved to the new Asset Importer data attachments (Issue #285), so the EmbeddedFont property will no longer exist in the long run. All that distinguishes Fonts would be the importer that is preferred for them, which is something that shouldn't be checked explicitly.
Is there a specific reason why you want to know what "kind" of Font you're dealing with?

Hahaha no prob. I'm expecting a good code thrashing. I respect that you take care of your code base :smile:
The reason I wanted to know was purely to not get the null checks wrong since the font has to be re-generated in some cases (eg size is changed)
But you'll see in the code...

Allright, now that the Offset PR is all wrapped up and merged, what will be the next steps? Apart from the readonly issue that prevents users from toggling kerning, is there anything else that's missing for getting your plugin to work in a first iteration?
The readonly issue can be solved by some more specialized FontPropertyEditor code and will no longer exist after #285, but there's no ETA on that, so a workaround might be a good thing until then.

I was thinking about this. The main reason you make the properties readonly is because you need an embedded font to regenerate the glyphs.
But for updating kerning you don't need to regenerate them. So how about excluding only the kerning property from becoming readonly. And on changing the kerning property the glyphs do not regenerate but the kerning arrays are recalculated.

Yep, that's what I was trying to say with the second paragraph ^^ I can look into this, or you can do if you want to take this as a chance for diving into this.
Overall, what's the state / roadmap for your plugin?

Have a look 😁
You can try it by just adding the .cs file to the main Duality project for the moment - until I make an actual package.

I have implemented a better readonly-switch in the FontPropertyEditor, so you can now switch kerning and various other properties on and off without an embedded .ttf Font. Let me know if that fixes the issue for your scenario.

Have a look :grin:

Tried it. :+1:

Also, nice work on the info page!
That said, I did stumble across an issue - space characters appear to be really, really small. When writing a text, you can sometimes barely see the difference between two words, or just two characters of a word. I took a quick look at your parsing code and it seems like you don't parse the xadvance attribute of a BMFont's glyphs, which should determine behavior in special cases where a char can be wider than its visual representation.
When looking at a BMFont file (especially in one with outlines and shadow effects), you can see that in almost all cases, xadvance is different from width, so I think that this needs to be addressed. Unfortunately, Duality Fonts do not distinguish between width and x-advancement, so that would require another core change, similar to the Y Offset one.
Currently, x advancement is determined by a glyph's Width - OffsetX, which would need to be replaced with a new AdvanceX field.

Ah, by the way: Your importer currently creates a Pixmap output as well as a Font output. It doesn't actually create it itself as far as I can see, but it does specify that it produces a Pixmap Resource as an output. This is not required! Instead of adding a predicted Pixmap output here, you should instead specify that you handled the pixmap's image source file! This will tell the system that it shouldn't ask other importers to handle the file, because you already did.

When looking at a BMFont file (especially in one with outlines and shadow effects), you can see that in almost all cases, xadvance is different from width, so I think that this needs to be addressed. Unfortunately, Duality Fonts do not distinguish between width and x-advancement, so that would require another core change, similar to the Y Offset one.

Cool no problem, I can add that easily

Ah, by the way: Your importer currently creates a Pixmap output as well as a Font output. It doesn't actually create it itself as far as I can see, but it does specify that it produces a Pixmap Resource as an output. This is not required! Instead of adding a predicted Pixmap output here, you should instead specify that you handled the pixmap's image source file. This will tell the system that it shouldn't ask other importers to handle the file, because you already did.

I actually handled that purely so that the pixmap file get's copied to the Media folder. I'm not sure it's required, but it is part of the source so I thought it was useful. I'll play with what you said.

I have implemented a better readonly-switch in the FontPropertyEditor, so you can now switch kerning and various other properties on and off without an embedded .ttf Font. Let me know if that fixes the issue for your scenario.

That's brilliant! I tried a bunch of things last night, but that property editor code is some complex stuff :laughing:  I couldn't figure out how to only disable certain properties.

That's brilliant! I tried a bunch of things last night, but that property editor code is some complex stuff :laughing:

Yep, not the most convenient and simple part of Duality :smile: Figured it'd be helpful if I just quickly looked into this.

I actually handled that purely so that the pixmap file get's copied to the Media folder. I'm not sure it's required, but it is part of the source so I thought it was useful. I'll play with what you said.

Completely agree! As long as you tell the system you're handling the input file, it should actually get copied, so that should be fine.
(I suspect the reason it is copied right now is because the system thinks no-one has handled the .png file, then forwards it to the Pixmap importer, which will take care of it. This should not be relied on though.)

Cool no problem, I can add that easily

Great! I'll be waiting for another Pull Request then. Let me know if you need anything.

Just merged my recent progress on #321 into the master branch. It is now possible to specify custom import parameters using IAssetImportEnvironment.GetOrInitParameter<T>. These parameters then become user-editable in the editor. Might be useful in order to make a BMFont import configurable, if there are options that would make sense.

Will check it out thanks. A bit sick at the moment but will get back into thigs soon :smile:

Completed issue #322, which means all Font import data has been moved to the new import parameters API in the new custom AssetInfo data attachments. It's backwards compatible for regular editor usage, but you will likely have to update your BMFont importer to match the new SetGlyphData method signature that was introduced.
There is also no longer any dependency on an EmbeddedTrueTypeFont property and it only exists for binary compatibility reasons.
Just an FYI so you know what's Font / import code has been up to recently :)

Awesome, can't wait to play with it. Will you be updating the nugets?

Definitely, but not yet. This is a big change and it comes with a lot of potential to break stuff. I'd like to keep this code-only for testing until I'm sure I haven't accidentally broken something crucial.

Rediscovered the issue during a general issue cleanup and I'd like to wrap this up. From reading some of the old comments, I'd assume that the importer is in a WiP state before the v2.x Font update was released. Meanwhile, there was more work done on the v3.0 development branch, adding support for specifying kerning pairs explicitly in the importer.
@mika76 So.. it's been a while, but if you are willing to pick up work on this again, we could check and see how it would make sense to proceed here:

Integrate it into master without the new kerning support?
Integrate it into develop-3.0 making use of the new kerning support?
Something else?

Alternatively, we could re-assign this issue to any willing contributor, who could approach it either from scratch, or make use of the work that was done before. My priority here would be to clear up which state this issue is in.

Hi @ilexp I'm not sure I have the time for this now - I am quite happy if someone want to take over and continue the work. I think it's a great addition although I'm not sure if anybody's using it or not...

@mika76 Alright, thanks for the quick response, and thank you for the previous work on this :) This is up for grabs then.
For maintenance reasons, let's move this into the core as well and re-approach this issue with the goal of submitting, iterating on and merging a Pull Request.
ToDo

Investigate the current state of the plugin, as well as compatibility with the latest v2.x of Duality.
Introduce the importer into the core / master branch.
Test whether it works, polish if / as needed and consider adjustments to bring it in line with the latest Duality version, should there be any required.
Prepare a Pull Request on this, iterate on it through review stages. Expect change requests, since core additions will be checked a lot more in detail than separate plugins.


Changed Title Tag on Quick Start Guide

Just realised there's a few like this. I'll consolidate them into 1 pull request


number_chars/2 unexpected syntax error.

It seems there is really some progress! The reason why looking at number_chars/2 is most effective is that it already needs (part of) Prolog's syntax. And thus this can be tested easily. Something new:
?- number_chars(N,"- 1").
N = -1. % expected
?- number_chars(N,"'-' 1").
2022/04/18 16:56:09 error(syntax_error(not_a_number), 'Not a number.') % unexpected ; expected as above

See this file for all the tests so far.

I fixed the parser and now it accepts '-' as a sign.
$ git status
On branch refine-number_chars
Your branch is up to date with 'origin/refine-number_chars'.

nothing to commit, working tree clean
$ go install github.com/ichiban/prolog/cmd/1pl
$ 1pl
Top level for ichiban/prolog (devel)
This is for testing purposes only!
See https://github.com/ichiban/prolog for more details.
Type Ctrl-C or 'halt.' to exit.
?- number_chars(N,"- 1").
N = -1.
?- number_chars(N,"'-' 1").
N = -1.


Feature: allow supressing some package init logics

When running unit test, some packages are annoying.Though we are not going to use it while testing, it still stops testing.
Example: vendor/github.com/lucas-clemente/quic-go/internal/handshake/unsafe.go
func init() {
	if !structsEqual(&tls.ConnectionState{}, &qtls.ConnectionState{}) {
		panic("qtls.ConnectionState not compatible with tls.ConnectionState")
	}
	if !structsEqual(&tls.ClientSessionState{}, &qtls.ClientSessionState{}) {
		panic("qtls.ClientSessionState not compatible with tls.ClientSessionState")
	}
}

I want to rewrite it into
func init() {
        if true {
            return
        }
	if !structsEqual(&tls.ConnectionState{}, &qtls.ConnectionState{}) {
		panic("qtls.ConnectionState not compatible with tls.ConnectionState")
	}
	if !structsEqual(&tls.ClientSessionState{}, &qtls.ClientSessionState{}) {
		panic("qtls.ClientSessionState not compatible with tls.ClientSessionState")
	}
}

A solution that does not require changing xgo, using shell to help patch and restore the source file:
#!/usr/bin/env bash

patch_file=vendor/github.com/lucas-clemente/quic-go/internal/handshake/unsafe.go
orig_content=$(cat "$patch_file")

function apply_patch_file {
    cat > "$patch_file" <<EOF
//go:build ignore
// +build ignore

$orig_content
EOF
}

function restore_patch_file {
  echo "$orig_content" > "$patch_file"
}

apply_patch_file
trap restore_patch_file EXIT

xgo test -mod=vendor -v ./...


Migrate to KeyEvent from RawKeyEvent

Description
This does a major migration for SuperEditor to use KeyEvent instead of using RawKeyEvent-based events.
The RawKeyEvent system will shortly be deprecated by the Flutter team, and this PR fixes SuperEditor to handle that deprecation.
Unfortunately, this requires changing the public interface of SuperEditor, since some of the keyboard event handlers need to now take KeyEvents instead of RawKeyEvents.
The RawKeyEvent-based system has had deprecation warnings ("this will be deprecated" in the comments, not actual @Deprecated annotations) for a couple of years, and Flutter is finally officially deprecating that system, since the replacement has proven to be robust.
@matthew-carroll I haven't bumped any version numbers or anything here, since I'm not quite sure how you'd like to proceed with that, or how you want to handle this breaking change.
Related Issues

https://github.com/flutter/flutter/issues/136419

Related PRs

https://github.com/flutter/flutter/pull/136854
https://github.com/flutter/flutter/pull/136677

Tests

Modified tests to match.

@gspencergoog is this only a breaking change in terms of API? Or are we losing behaviors that we depend upon? If it's just an API breaking change, can you please make the replacements and ensure all tests still pass?
@miguelcmedeiros @brian-superlist @Jethro87 @jmatth - Please be aware of this PR from the Flutter team

PS - Goldens are expected to fail at the moment due to some minor adjustments for mobile controls. But I think all other tests should be made to pass.

As far as I can tell, I don't think you're losing any behaviors that you depend upon.
To be clear, removing the old system does remove some functionality (things having to do with specific platform implementations of key events in the RawKeyEventData subclasses), but I don't think you're using any of them.
Tests did pass locally for me.  I'll look into the ones that aren't passing on CI.

Oh, actually, I know why they're failing: they depend on a (breaking) change that hasn't landed yet in Flutter: https://github.com/flutter/flutter/pull/136854

Ok, can you let us know in this PR when the other change is merged? We wouldn't want to merge a PR that breaks us against master.

Ok, can you let us know in this PR when the other change is merged? We wouldn't want to merge a PR that breaks us against master.

Yes, of course.  How exactly do you want to do "the dance" here?  At some point, either when I merge the breaking change to the framework, or when we merge this change here before the framework is updated, your repo won't build against master for a short period of time. Once either one happens, presumably you wouldn't be publishing the package until they are in sync again and passing.
It sounds like you'd prefer me to push the framework change first, and then update this change to validate against the new framework master before you commit this change here. Does that sound right?

It looks like there are about 50 different APIs in SuperEditor that take a RawKeyEvent that I've converted to use KeyEvent here, but in order to land the change in the Framework, I'll either need to convert all of those to dynamic (and have duplicate code to handle either), commit them here, and then convert them all back again and remove the duplicate code once the framework PR lands, or we'll need to disable the SuperEditor customer tests temporarily until both the Framework change and this PR land.
Would you be OK with disabling the customer tests temporarily to save us some time and avoid the churn in your code?

Would you be OK with disabling the customer tests temporarily to save us some time and avoid the churn in your code?

Yes, as long as we re-enable them ASAP.

Yes, the plan would be to re-enable them ASAP (we want them enabled as much as you do!)
I wouldn't disable them until both of the PRs were ready and approved, and then I'd submit the framework PR, and then rebase this PR, submitting it (with your cooperation) as soon as the presubmits passed, and then re-enable the customer tests right after that.

@matthew-carroll Okay, the Flutter change is ready to be committed.  Once you approve https://github.com/flutter/tests/pull/312, I'll submit that, let the presubmit pass on https://github.com/flutter/flutter/pull/136854 and then submit it.  Once that's submitted, I'll push an empty change to this PR to re-run the presubmits. Then, once this PR is approved, you can submit it and I'll re-enable the customer_testing tests with an update ref. Sound good?

It looks like 1) there are the (expected, I think) golden file failures, and 2) the deploy tests are failing because it's trying to build this PR with the stable branch.  Is that expected?

@angelosilvestre @rutvik110 - Please do a review pass on this PR to see if you find anything that I missed.
@miguelcmedeiros @jmatth @Jethro87 - I've done an initial review of this PR from the Flutter team. Once that the review completes, I plan to merge this in. So if you have any input, now is the time.

LGTM!

The tests didn't pass because of  BlinkTimingMode being missing (no idea why), and because the symbols in the Flutter master branch on HardwareKeyboard that I'm using are missing.  They are available in the master branch, so is there a ref or version or something that needs to be updated so that the tests run against that?

The tests didn't pass because of some golden test failures, and BlinkTimingMode being missing (no idea why), and because the symbols in the Flutter master branch on HardwareKeyboard that I'm using are missing. They are available in the master branch, so is there a ref or version or something that needs to be updated so that the tests run against that?

@gspencergoog I see all tests as green except the goldens. Which failures are you referring to?

I see all tests as green except the goldens. Which failures are you referring to?

The suspicious-kepler ones, for example: https://app.netlify.com/sites/suspicious-kepler-28398e/deploys/6564fc545c05840008faeb79

Looks like all of the golden tests that are failing here are also failing on main against stable (with seemingly identical differences), so we can probably ignore those.

@gspencergoog I think we're ignoring the marketing website CI checks. We should probably fix that at some point, but I don't think we need to worry about it here. All relevant tests appear to be passing. I'll do another PR review when I get a minute.

@angelosilvestre if you'd like to try logging Flutter master commits, here's one to track.


Zoom in and out using mouse scroll wheel

it does what the buttons do but using the scroll wheel, closes #6168

I purposedly removed this because it looks awful, until we implement proper
multisampled AA there is no hope of implementing proper zoom.
On Sun, Aug 28, 2016 at 2:43 PM, Daniel Ramirez<EMAIL_ADDRESS>wrote:

it does what the buttons do but using the scroll wheel, closes #6168
https://github.com/godotengine/godot/issues/6168
You can view, comment on, or merge this pull request online at:
https://github.com/godotengine/godot/pull/6317
Commit Summary

Zoom in and out using mouse scroll wheel

File Changes

M scene/gui/graph_edit.cpp
https://github.com/godotengine/godot/pull/6317/files#diff-0 (4)

Patch Links:

https://github.com/godotengine/godot/pull/6317.patch
https://github.com/godotengine/godot/pull/6317.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/pull/6317, or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z25H7_o0iErylbMe580FTDSCBKBmFks5qkcjOgaJpZM4Ju-aE
.

but this is just the same as the buttons so i guess this has no problems, since the functionality is already there. this is just simpler to use, later when the implementation changes it will work for both modes, buttons and scroll wheel

buttons are there to discourage zooming unless you really need it :)
On Sun, Aug 28, 2016 at 3:25 PM, Daniel Ramirez<EMAIL_ADDRESS>wrote:

but this is just the same as the buttons so i guess this has no problems,
since the functionality is already there. this is just simpler to use,
later when the implementation changes it will work for both modes, buttons
and scroll wheel
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/pull/6317#issuecomment-242990588,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z23ntw1D-Jn2SVy4hOKUTs8ohxAYzks5qkdKGgaJpZM4Ju-aE
.

I think also a minimap should reduce the need for zooming
On Mon, Aug 29, 2016 at 12:00 PM, Juan Linietsky<EMAIL_ADDRESS>wrote:

buttons are there to discourage zooming unless you really need it :)
On Sun, Aug 28, 2016 at 3:25 PM, Daniel Ramirez<EMAIL_ADDRESS>wrote:

but this is just the same as the buttons so i guess this has no problems,
since the functionality is already there. this is just simpler to use,
later when the implementation changes it will work for both modes, buttons
and scroll wheel
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/pull/6317#issuecomment-242990588,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z23ntw1D-Jn2SVy4hOKUTs8ohxAYzks5qkdKGgaJpZM4Ju-aE
.

well i agree zoomed graph looks awful, also i posted my suggestion about the minimap on an issue


java.lang.NoClassDefFoundError: org/spark_project/guava/util/concurrent/RateLimiter

I'm using spark-dynamodb_2.11:0.2.2 from https://oss.sonatype.org
I got no RateLimiter class error when I write to a dynamo table as below,
newItemsDs.write.dynamodb("table-name")
I use the jar from https://mvnrepository.com/artifact/com.google.guava/guava/26.0-jre as I cannot find org.spark_project.guava used in the table connector.
TableConnector.scala:30:import org.spark_project.guava.util.concurrent.RateLimiter

Hi yeungp
I have fixed the guava dependency to 14.0.1 (https://search.maven.org/artifact/com.google.guava/guava/14.0.1/bundle). This is the version used by the latest Spark release (2.3.1).
Try and add "com.google.guava"%"guava"%"14.0.1" to your project. The fix will be on version 0.3.0 of spark-dynamodb, which should be available on Maven by tomorrow.
I hope this helps,
Jacob

#5

In case of using gradle for the build tool, you might specify guava version in this way.
configurations.all {
    // https://github.com/apache/spark/blob/master/pom.xml#L512
    resolutionStrategy.force "com.google.guava:guava:14.0.1"
}


spelling mistakes in ui

there are lot of spelling mistakes in ui of landing page and signup page

solved the issue in the recent PR(#64 )


myfixip + mod security

It seems myfixip doesn't set the variables used by mod security when parsing the agent IP. I'm not sure if it's an issue on myfixip or mod security. I tried to use remoteip as workaround but that didn't work.
Apache 2.4.7
Mod Security 2.7.7
<IfModule mod_myfixip.c>
    RewriteIPResetHeader on
    RewriteIPAllow <IP_ADDRESS>/16 <IP_ADDRESS>
</IfModule>

<IfModule mod_remoteip.c>
    RemoteIPHeader X-Cluster-Client-Ip
    # Just <IP_ADDRESS>/16 listed on file
    RemoteIPInternalProxyList /etc/apache2/trusted-proxies.lst
</IfModule>

Mod security logs showing the correct external IP address on apache logs:
Dec  7 19:55:28 dev www_dev[5836]:  [client 54.165.XXX.XXX] ModSecurity: Access denied with code 403 (phase 2). Pattern match ..."

Mod security audit logs parsing the load balancer IP instead.
--90491855-A--
[07/Dec/2015:19:35:54 +0000] VmXfmn8AAAEAAANSL4MAAABw <IP_ADDRESS> 14882 <IP_ADDRESS> 443
--90491855-B--
GET /?test_xss=%3Cscript%3Ealert(1)%3C/script%3E HTTP/1.1
Host: devl
Connection: keep-alive
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.73 Safari/537.36
Accept-Encoding: gzip, deflate, sdch
Accept-Language: pt-BR,pt;q=0.8,en-CA;q=0.6,en;q=0.4,en-US;q=0.2
X-Cluster-Client-Ip: 54.165.XXX.XXX

you don't need mod_remoteip, fixip and remoteip are similar, but different implementation and concept...

@ggrandes But remoteip doesn't read PROXY headers does it ? It seems mod security reads a field that myfixip doesn't set, but remoteip does. Honestly I'm not sure.

@ericofusco correct, remoteip don't understand PROXY protocol. But mod_remoteip don't set any header, only read...
A) Are you sure that mod_security need one http-header?
B) What config are you using in mod_security?

Hi @ggrandes ,
Sorry the delay.
A) Mod security doesn't read any HTTP headers, but it seems mod security had some work done to work with remoteip to set a new remote_addr.
B) I'm using default configuration, nothing unusual.
Check these lines on mod security, maybe I can make some changes to mod security to make it work with myfixip and/or we can make changes to myfixip to work with mod security.
Mod Security (2.9.0);
apache2/mod_security2.c
    static const char *const postread_beforeme_list[] = {
        "mod_rpaf.c",
        "mod_rpaf-2.0.c",
        "mod_extract_forwarded.c",
        "mod_extract_forwarded2.c",
        "mod_remoteip.c",
        "mod_custom_header.c",
        "mod_breach_realip.c",
        "mod_breach_trans.c",
        "mod_unique_id.c",
        NULL
    };

apache2/re_variables.c
#if !defined(MSC_TEST)
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 3
    if (ap_find_linked_module("mod_remoteip.c") != NULL) {
        if(msr->r->useragent_ip != NULL) msr->remote_addr = apr_pstrdup(msr->mp, msr->r->useragent_ip);
        return var_simple_generate(var, vartab, mptmp, msr->remote_addr);
    }
#endif
#endif

Clearly there is a dependence on remoteip to set another remote_addr, but I believe myfixip doesn't set useragent_ip either. Maybe changes need to be done on both ends.
Please let me know your thoughts.
Thanks.

Ok, try this change in mod_myfixip.c (version 1.3), without load mod_remoteip:
- ap_hook_post_read_request(post_read_handler, NULL, NULL, APR_HOOK_FIRST);
+ ap_hook_post_read_request(post_read_handler, NULL, NULL, APR_HOOK_REALLY_FIRST - 1);

give me your feedback!

@ericofusco I released a new version (1.4), try with this :-)

Hey @ggrandes,
Thanks a lot for the update.
Unfortunately it didn't work. Mod security still parses the load balancer IP. Although I changed modsecurity and updated the condition from the block I posted before to the one below.
if (ap_find_linked_module("mod_remoteip.c") != NULL || ap_find_linked_module("mod_myfixip.c") != NULL)

That did the trick. Does that make sense to you ? I believe myfixip only changes useragent_ip just like remoteip. If that's the take I'll just do pull request to mod security to load myfixip before and add this condition.
Please let me know your thoughts.
Thanks.

Hi @ericofusco , this seem correct aproach!

Thanks a lot @ggrandes. I'll see if I can push this update to modsecurity. Task can be closed.


Landlord immigration check FC feedback

Closes #1792 and #1793.
In addition to the above, this PR removes a no longer used named_person_of_eea_switzerland_person node and adds a test for the modified flow.
Note: this smartanswer is in draft state, so it's safe to merge to master.
Original PR description
From factcheck:

removing Channel Islands route (will be caught by later questions)
changing outcome for non-eea family members to a later outcome (because they won't have the other documents)
fixing a typo

Other than my suggestion about splitting the first commit, this LGTM.


Add chadb to Contributors List

@chadb4184, I'm quite elated about your PR. I wanna evolve this project to addresses various problems faced by first-time contributors. I'd love to learn about your journey in open source community, the problems, pain points you had etc.
Could you explain how you felt when you went through the tutorial, made a PR and learned that I merged it?

Felt awesome. I tried going through the tutorial using the command line interface in Windows a few days ago. Downloaded GitKraken and tried going through that tutorial. I'm having trouble now though, syncing the repository.
When I go to checkout the master (under upstream) it prompts to create a new branch. Is this correct? It's not part of the tutorial.
Sent from my iPhone

On Jul 18, 2017, at 3:33 PM, Roshan Jossey<EMAIL_ADDRESS>wrote:
Merged #276.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Hey @chadb4184, I'm not sure about synching part in GitKraken. I'll check what's happening there and get back to you.
I’d urge you to continue contributing to other open source projects.
We also have a slack team to help/mentor people trying to contribute to open source projects.
You can join the team by going to https://firstcontributions.herokuapp.com


3.6

Backport from master (JGRP-2364)

What's this? A merge from 3.6 branch to master? That's not the way it works; merging is usually from upstream (branch off of master) onto an (older) branch.
Also: 238 files changes? :-)


Change input type of period field to number

Description of change
Change the input type of the period field to be number instead of text since it's intended that numbers be entered.

this makes total sense ..just curious but were there any downstream hacks/fixes in view etc for this that this change would now make obsolete?

Not AFAICT, the big issue is I don't know if this impacts anything major. Was going to ping patrick once this was ready to review

Weirdly I think we actually don't want to do this: https://technology.blog.gov.uk/2020/02/24/why-the-gov-uk-design-system-team-changed-the-input-type-for-numbers/ and https://design-system.service.gov.uk/components/text-input/#avoid-using-inputs-with-a-type-of-number

That's what I was worried about, annoying but understandable. Thanks for that.


Use take instead of two commands

the take command will call mkdir and cd in one command.

Thanks for the suggestion, but we are going to close this for now as we believe take isn't generally available.


Sort out 'this'

Problem: 'this' is currently a keyword but needs to be able to be used where a variable reference can be e.g.
return this

let other be copy this with ...

call foo(this)

Probably we need to change this from being a keyword to being a special case symbol.

Closed as incorporating into #670


refactor: Exposing Neo4j Driver Configs

This PR aims at exposing Neo4j Driver Configs as env variables.
Following Variables have been exposed:

NEO4J_MAX_CONNECTION_POOL_SIZE
NEO4J_MAX_CONNECTION_ACQUISITION_TIMEOUT_IN_SECONDS
NEO4j_MAX_CONNECTION_LIFETIME_IN_HOURS
NEO4J_MAX_TRANSACTION_RETRY_TIME_IN_SECONDS

Checklist

[X] The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
[X] Links to related issues (if applicable)
[X] Tests for the changes have been added/updated (if applicable)
[X] Docs related to the changes have been added/updated (if applicable)

Appreciate your contribution!


Fix #287 : Support manifest.json

Using a client-made curseforge modpack
If you use something like curseforge, you may end up creating/using modpacks that do not
contain server mod jars. Instead, the curseforge setup has manifest.json files, which
will show up under /data/FeedTheBeast/manifest.json.
To use these packs you will need to:

Specify the manifest location with env var MANIFEST=/data/FeedTheBeast/manifest
Pick a relevant ServerStart.sh and potentially settings.cfg and put them in /data/FeedTheBeast

An example of the latter would be to use https://github.com/AllTheMods/Server-Scripts
There, you'll find that all you have to do is put ServerStart.sh and settings.cfg into
/data/FeedTheBeast, taking care to update settings.cfg to specify your desired version
of minecraft and forge. You can do this in the cli with something like:
$ wget https://raw.githubusercontent.com/AllTheMods/Server-Scripts/master/ServerStart.sh
$ wget https://raw.githubusercontent.com/AllTheMods/Server-Scripts/master/settings.cfg
$ vim settings.cfg #update the forge version to the one you want.

Let me know if there's anything else you'd like

This is now pushed to Docker Hub.

I'm embarrassed to say that while I tested this, on my own version, apparently a bug slipped in. I should have cleared my server dir properly. ugh. -_-
at least I tested your version and found the bugs, though! ;-)


Certain GLTF imports have broken normals

Originally posted by @rafrafek in https://github.com/godotengine/godot/issues/85406#issuecomment-2325031951
I tried to reproduce it in an MRP and now instead of having dark strips in becomes entirely darker when rotating:
https://github.com/rafrafek/normals-demo

I have another mesh that has some broken normals on import.  Disabling compression didn't seem to fix it this time.

This is coming straight from importing a .blend file:
revolutionary_revolver.zip
Checked the gltf file generated and that seems good in blender, so I think it's purely a Godot issue, not a GLTF export issue.
revolutionary_revolver-4f30941feabb7df2225b6127bb3c60d2.zip


docker-compose with c-lightning container?

lnbook (https://github.com/lnbook/lnbook/blob/develop/04_node_client.asciidoc) has a c-lightning docker container that I was trying to see if it could be composed in the plebnet-playground environment to connect with existing lnd nodes over signet. Has anyone tried composing plebnet-playground with c-lightning instead of lnd?

starting a  c-lighting branch for this!

yeah I had the container running, it just needed to be hooked up with the new yaml template setup, should make it easier to integrate now.  I had tested it back then, worked fine. Just all the tooling people have been working on has been LND so haven't spent much time on it.

I'm having problems linking the clightning container to bitcoind. Any ideas?
bitcoin-cli -rpcconnect playground-bitcoind -rpcport 38332 -rpcuser bitcoin -rpcpassword bitcoin
error: Could not connect to the server :8332 (error code 1 - "EOF reached")

I know I had it running an synced up but might have been the clightning branch over in cluster repo. I know Randy also had it running for a short while

what's the status on this? Should i start a PR with my c-lightning branch or did @rsafier worked on this separately?

let me go review that branch I can update image to what I found works well and you can get it integrated into the template.


What's this? And how to use

please guide me. maybe it's what i'm looking for

look for uabe tutorials its basically the same thing. if you cant do that much....i dont think anyone can help you.


Fallback for when custom layout is missing

Describe the feature
Hi. I have an application where the layout selected is based on a request to an API. Sometimes that API might request a layout that doesn't exist (yet or it wont ever exist) and I want to show an error page or a fallback. This doesn't seem to exist currently.
I have read most of the top results on google and I've read through any related issues on github that I could find.

Ideally would be a config and/or a named layout you can create.
Additional information

[ ] Would you be willing to help implement this feature?
[ ] Could this feature be implemented as a module?

Final checks

[X] Read the contribution guide.
[X] Check existing discussions and issues.

We think this API would be a good idea, which would require opting-in:
<NuxtLayout fallback="default">
  <NuxtPage />
</NuxtLayout>

@danielroe I think that makes a lot of sense. Thanks!


stickler.yml added

Your eslint configuration output the following error:
Cannot find module 'eslint-config-airbnb'
Referenced from: /tmp/healqq/traintrack/1/./frontend/.eslintrc


Add support for ':' in function names to support Java EL style functi…

…ons with namespace

I like the idea of having more compatibility with Java EL (which I only just learned about today!) But unfortunately this change would introduce some ambiguity with ternaries.
Ternaries are, by design, capable of having no else-case in this library. So true ? foo would correctly evaluate to the value of foo.
But consider this case;
false ? foo:length()
This library ignores whitespace, so there's no semantic difference between false ? foo:length() and false ? foo : length().
Is the user trying to specify "if the condition is true, take foo, otherwise call length()"
Or should it be interpreted as "if the condition is true, take foo:length()"
Currently, on master branch, you can use backslashes like false ? foo\:length() to make it unambiguous. But i'm not sure if that fits your use cases.

I added a test in https://github.com/Knetic/govaluate/commit/74a069a4133fc21168c68f227f5d8c5c1c74f2a0 to make sure what i said above was correct, and unfortunately I can't accept this PR because of it. Other languages can make it work because they do not allow the form of ternaries where there is no else case (which might be better called a binary? I guess?) such as isTrue() ? doSomething(), but this library supports that case.
The workaround to escape : in function names with backslashes will still work for your use cases though.
Thanks for the PR though! I appreciate the time spent on it, even if i can't accept.


Prototype error for CMessage.GetParamsColon on Python

Version: 1.8.1
The API bindings appear unhappy in Python.
Traceback (most recent call last):
  File "/.../modules/blah.py", line 246, in OnPrivCTCPMessage
    self.PutModule("ParamsColon 1: " + str(message.GetParamsColon(1)))
  File "/.../znc/modpython/znc_core.py", line 3588, in GetParamsColon
     return _znc_core.CMessage_GetParamsColon(self, uIdx, uLen)
TypeError: Wrong number or type of arguments for overloaded function 'CMessage_GetParamsColon'.
  Possible C/C++ prototypes are:
    CMessage::GetParamsColon(unsigned int,unsigned int) const
    CMessage::GetParamsColon(unsigned int) const

Relevant lines in /.../znc/modpython/znc_core.py are as follows:
def GetParamsColon(self, uIdx: 'unsigned int', uLen: 'unsigned int'=-1) -> "CString":
    return _znc_core.CMessage_GetParamsColon(self, uIdx, uLen)

Clearly an unsigned int cannot be -1 (why that definition on C++ side anyways?!), so my guess is that some code (SWIG?) chokes on this.
I have tried setting uLen to None in the hope it would fall back to the 1-parameter version, but no dice.
Temporary workaround for anyone who needs the default for that parameter:
import _znc_core
self.PutModule("ParamsColon 1: " + str(_znc_core.CMessage_GetParamsColon(message, 1)))

I don't know what the proper way is to resolve this. Regenerate SWIG? Fix the original class definition with a proper overload somehow? I leave it up to the experts. 😄

Can you show your code?

It is pretty much in the traceback, which is why I didn't post it. It is just a global module, with the hook in it, and the line of the exception.
    # virtual EModRet CModule::OnPrivCTCPMessage ( CCTCPMessage& Message )
    def OnPrivCTCPMessage(self, message):
        self.PutModule("ParamsColon 1: " + str(message.GetParamsColon(1)))


🏗 PR ➾ 1168 ligo improvements jcc

🏗 PR ➾
Fixes #1168
🪂 Pre-Merge Checklist

[x] 🐬 I have commented my code, particularly in hard-to-understand areas
[ ] 🦀 Automated tests have been written and added to this PR
[ ] 🤿 Corresponding changes have been made to all documentation
[ ] 🏖 New and existing unit tests pass locally and in CI
[ ] 🔱 The test plan has been implemented and verified by an SDET
[x] 🏄♂ Myself and others have built this branch and done manual testing on the change
[x] ⛵ A summary describing the change has been written for release notes

🪁 Description
We did not have means to compile storages and parameters in the LIGO plugin. This change adds those features by introducing a naming convention. e.g. taq compile hello.mligo will compile the contract like before, but taq compile hello.storages.mligo and taq compile hello.parameters.mligo will compile the LIGO storage and parameter expressions defined in those files respectively. Furthermore, contract compilation will automatically perform compilation of its associated "storages" and "parameters" files.
🎢 Test Plan
Please describe the testing strategy for this change

🛸 Type of Change

[ ] 🐟 Minor change (non-breaking change of very limited scope)
[ ] 🦑 Bug fix (non-breaking change which fixes an issue)
[x] ✨ New feature (non-breaking change which adds functionality)
[ ] 🌊 Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] 🐳 Major refactor (breaking changes and significant impact to the codebase)

I'm trying to compile a single contract test.jsligo and am getting an error:
❯ taq compile test.jsligo

=== For test.jsligo ===
An internal error ocurred. Please, contact the developers.
lexing: empty token.

Note: storage file associated with test.jsligo can't be found. You should create a file named test.storages.jsligo in the same directory as test.jsligo and define initial storage values as a list of LIGO variable definitions. e.g. "let STORAGE_NAME: storage = LIGO_EXPR" for mligo
Note: parameter file associated with test.jsligo can't be found. You should create a file named test.parameters.jsligo in the same directory as test.jsligo and define parameter values as a list of LIGO variable definitions. e.g. "let PARAMETER_NAME: parameter = LIGO_EXPR" for mligo
┌─────────────┬──────────────┐
│ Contract    │ Artifact     │
├─────────────┼──────────────┤
│ test.jsligo │ Not compiled │
└─────────────┴──────────────┘

In the default case, we should just compile the contract/sourcefile as is and maybe mention in a warning or stdOut that there was no storage/param file found (as above)

I'm trying to compile a single contract test.jsligo and am getting an error:
❯ taq compile test.jsligo

=== For test.jsligo ===
An internal error ocurred. Please, contact the developers.
lexing: empty token.

Note: storage file associated with test.jsligo can't be found. You should create a file named test.storages.jsligo in the same directory as test.jsligo and define initial storage values as a list of LIGO variable definitions. e.g. "let STORAGE_NAME: storage = LIGO_EXPR" for mligo
Note: parameter file associated with test.jsligo can't be found. You should create a file named test.parameters.jsligo in the same directory as test.jsligo and define parameter values as a list of LIGO variable definitions. e.g. "let PARAMETER_NAME: parameter = LIGO_EXPR" for mligo
┌─────────────┬──────────────┐
│ Contract    │ Artifact     │
├─────────────┼──────────────┤
│ test.jsligo │ Not compiled │
└─────────────┴──────────────┘

In the default case, we should just compile the contract/sourcefile as is and maybe mention in a warning or stdOut that there was no storage/param file found (as above)

What's the content of test.jsligo? Seems like an error from the LIGO compiler itself I believe. Yes it compile the contract regardless of whether it finds the storages and parameters files or not. It's not compiling here due to a LIGO error.


Allow plugin setting (default) values to reference pipeline plugin extras using generic env vars

Migrated from GitLab: https://gitlab.com/meltano/meltano/-/issues/2349
Originally created by @DouweM on 2020-09-09 21:02:44

This will enable us to change the default value of the target-postgres and target-snowflake schema setting from $MELTANO_EXTRACTOR_NAMESPACE to $MELTANO_EXTRACT__PREFERRED_SCHEMA.

View 2 previous comments from the original issue on GitLab


TypeScript error in editor when configuring Drizzle plugin in knip.config.ts

Prerequisites

[X] I'm using the latest version
[X] I've read the relevant documentation
[X] I've searched for existing issues
[X] I've checked the list of known issues
[X] I've read the issue reproduction guide

Reproduction url
https://stackblitz.com/edit/github-hyliy9?file=knip.config.ts
Reproduction access

[X] I've made sure the reproduction is publicly accessible

Description of the issue
When using a typed config file, the type for the Drizzle plugin does not seem to exist:
Object literal may only specify known properties, and 'drizzle' does not exist in type '{ exclude?: ("dependencies" | "exports" | "files" | "devDependencies" | "optionalPeerDependencies" | "unlisted" | "binaries" | "unresolved" | "types" | "nsExports" | "nsTypes" | "duplicates" | "enumMembers" | "classMembers")[] | undefined; ... 98 more ...; workspaces?: Record<...> | undefined; }'.ts(2353)

The plugin itself works fine, though, I only noticed an issue with the types.

:rocket: This issue has been resolved in v5.36.6. See Release 5.36.6 for release notes.
Using Knip in a commercial project? Please consider becoming a sponsor.

@webpro thanks for resolving so quickly!


What font is used in the screenshots?

Great theme. I also really like the font you're using in the screenshots, what is it?
Thanks!

Hi. Thanks for the kind words.
The font is the Fira Code Nerd Font.
For the screenshot I'm using the medium width in Konsole.
I don't like the @ and the & characters of the default settings so I use the variants glyphs (ss03 and ss05) changed with this tool: FontFreezer


feat(PoC): add collapsible feature to Sidebar

FX-1855
Description
PoC for a collapsible sidebar.
How to test

FIXME: Add the steps describing how to verify your changes

Screenshots

Before.
After.

Insert screenshots or screen recordings
Insert screenshots or screen recordings

Review

[ ] Read CONTRIBUTING.md and Component API principles
[ ] Annotate all props in component with documentation
[ ] Create examples for component
[ ] Ensure that deployed demo has expected results and good examples
[ ] Ensure that tests pass by running yarn test
[ ] Ensure that visuals tests pass by running yarn test:visual. If not - check the documentation how to fix visual tests
[ ] Ensure the changed/created components have not caused accessibility issues. How to use accessibility plugin in storybook.

PR commands

List of available commands:

@toptal-bot run all - Run whole pipeline
@toptal-bot run build - Check build
@toptal-bot run visual - Run visual tests
@toptal-bot run deploy:documentation - Deploy documentation
@toptal-bot run package:alpha-release - Release alpha version

PR Review Guidelines

When to approve? ✅
You are OK with merging this PR and

You have no extra requests.
You have optional requests.

Add nit: to your comment. (ex. nit: I'd rename this variable from makeCircle to getCircle)

When to request changes? ❌
You are not OK with merging this PR because

Something is broken after the changes.
Acceptance criteria is not reached.
Code is dirty.

When to comment (neither ✅ nor ❌)
You want your comments to be addressed before merging this PR in cases like:

There are leftovers like unnecessary logs, comments, etc.
You have an opinionated comment regarding the code that requires a discussion.
You have questions.

How to handle the comments?

An owner of a comment is the only one who can resolve it.
An owner of a comment must resolve it when it's addressed.
A PR owner must reply with ✅ when a comment is addressed.

Since the original PR is here: https://github.com/toptal/picasso/pull/2610, I will close this to decrease ambiguity.


model_kwargs_cd is always a copy from model_kwargs, is it a bug?

Thanks for your awesome work.
I find model_kwargs_cd is always a copy from model_kwargs in while loop of vcd_sample file. In this case, the past_key_values of normal input and past_key_values of distorted input are same. Is it a bug?

Two pieces of code with logical conflict
model_kwargs_cd = model_kwargs.copy()

## cd_comments: update model_kwargs_cd for contrastive decoding
if use_cd:
    model_kwargs_cd = self._update_model_kwargs_for_generation(
        outputs_cd, model_kwargs_cd, is_encoder_decoder=self.config.is_encoder_decoder
    )

@lixin4ever @hangzhang-nlp  @LengSicong

Hi, thanks for your interest in our work.
For your concern, please refer to this issue.

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

It seems that the function prepare_inputs_labels_for_multimodal  will utilize the past_key_values to compute the logits when past_key_values exists and 'input_ids' is single, hence I think the past_key_values should not be equal between normal input case and distorted input case.
https://github.com/DAMO-NLP-SG/VCD/blob/master/experiments/llava/model/llava_arch.py#L94

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

same confusion, it seems that the distorted image will only be used for the generation of first predicted token:
        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step
        if inputs_embeds is not None and past_key_values is None:
            model_inputs = {"inputs_embeds": kwargs.get("images_cd")}
        else:
            model_inputs = {"input_ids": input_ids}

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

same confusion, it seems that the distorted image will only be used for the generation of first predicted token:
        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step
        if inputs_embeds is not None and past_key_values is None:
            model_inputs = {"inputs_embeds": kwargs.get("images_cd")}
        else:
            model_inputs = {"input_ids": input_ids}

This problem can be fixed by ignoring the copy operation, and the experimental results will not be affected in POPE evaluation, since the output token is single ('Yes'or 'No').
(https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L91)(https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L152)

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

same confusion, it seems that the distorted image will only be used for the generation of first predicted token:
        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step
        if inputs_embeds is not None and past_key_values is None:
            model_inputs = {"inputs_embeds": kwargs.get("images_cd")}
        else:
            model_inputs = {"input_ids": input_ids}

This problem can be fixed by ignoring the copy operation, and the experimental results will not be affected in POPE evaluation, since the output token is single ('Yes'or 'No'). (https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L91)(https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L152)

So the copy operation is not correct and needs to be deleted to make the contrastive decoding work for the whole sequence, right?

Actually, I have mentioned the problem of the same past_key_values in #7 and it has confused me for a really long time... So can I understand in this way that model_kwargs and model_kwargs_cd need to save the past_key_values separately, right?

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

same confusion, it seems that the distorted image will only be used for the generation of first predicted token:
        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step
        if inputs_embeds is not None and past_key_values is None:
            model_inputs = {"inputs_embeds": kwargs.get("images_cd")}
        else:
            model_inputs = {"input_ids": input_ids}

This problem can be fixed by ignoring the copy operation, and the experimental results will not be affected in POPE evaluation, since the output token is single ('Yes'or 'No'). (https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L91)(https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L152)

So the copy operation is not correct and needs to be deleted to make the contrastive decoding work for the whole sequence, right?

I think so. However, this is my personal opinion, it would be best if the author could respond to this question.

#7 comments I think it does not matter. you can find the process in llava_arch.py -> prepare_inputs_labels_for_multimodal. It does not resolve my doubts.

The past_key_values passed in forward function are same in normal input case and distorted input case.
I want to use vcd in Qwen-vl. I have checked the related files, but there is not similar process likes prepare_inputs_labels_for_multimodal.

I have the same question, I found the next_token_logits and next_token_logits_cd will be equal when the past_key_values are same in forward function (That's easy to understand, the input_ids is also same)
In this way, the diffs = (1+cd_alpha)*next_token_logits - cd_alpha*next_token_logits_cd will be invalid.

same confusion, it seems that the distorted image will only be used for the generation of first predicted token:
        # if `inputs_embeds` are passed, we only want to use them in the 1st generation step
        if inputs_embeds is not None and past_key_values is None:
            model_inputs = {"inputs_embeds": kwargs.get("images_cd")}
        else:
            model_inputs = {"input_ids": input_ids}

This problem can be fixed by ignoring the copy operation, and the experimental results will not be affected in POPE evaluation, since the output token is single ('Yes'or 'No'). (https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L91)(https://github.com/yfzhang114/LLaVA-Align/blob/main/vcd_utils/vcd_sample.py#L152)

Hi all, thanks a lot for your interest in our work and active discussions!
We are sorry that we made this mistake when cleaning the open-source code.
To make it clear:

The VCD mechanisms require two sets of past_key_values during the inference, one for the original image input and the other for the distorted image.
As explained by @darkpromise98 (thanks!:)), this problem will not affect the POPE result since there is only one output token.
We made this mistake only for the open-sourced cleaned codes. The experimental results reported in our paper follow the correct logic and are guaranteed to be faithful and reproducible.
We will correct the public codes as soon as possible. Instead of removing the copy operation directly, the copy should be moved before the while true loop only for the generation of the first token.


Examples cleanup

This moves and cleans up the 'Journeys' notebooks. See #558
It also addresses redundant 'Docs' and 'Example' in file names, see #488

@sbenthall,
Have spent a little while looking through this and it looks great.  Merging it now, because I'm hoping to move Bayer-Luetticke from "examples" to REMARK soon, and don't want to mix that with your revamp of the examples.


Added netscape.javascript package.

If interacting with JavaScript in a WebView one needs netscape.javascript.JSObject and netscape.javascript.JSException, hence it would be useful to export the netscape.javascript package.

Thank you :)

Hi.
You are welcome.
Thanks for merging it so quickly.
Colly
On Friday, 28 December 2018, 20:51:01 GMT, Edvin Syse<EMAIL_ADDRESS>wrote:

Thank you :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

My pleasure. I've also pushed version 8.0.3 to Maven Central now.

Hi. That's great. Thanks Colly
On Saturday, 29 December 2018, 10:56:17 GMT, Edvin Syse<EMAIL_ADDRESS>wrote:

My pleasure. I've also pushed version 8.0.3 to Maven Central now.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.


Push Version 2.0 to npm

I'd like to use version 2.0 in my app.  I'm currently pulling from npm though.  Any idea when 2.0 will be published to npm for production use?

Incoming today!

Excellent!  Thanks for the speedy update.

Done and done!

I'll verify this is fixed with 2.0 and open it again if it is not.  Thanks for the hard work!


Setup Organization Managers

add the required features so that an organization manager can manage an organization
Details
add field to orgs manager which is of type user. this user then has permissions to manage their org
Requirements

[x] A manager can't change their own manager status
[x] A manager has full access to an organization and all teams (only organization model, not like super admin. they will still need to assign themselves permissions)

added 3m of time spent

moving to v1.0.0 as this is required for org owners to manage their orgs.

mentioned in commit 33b10f710987da19d7abcf69d3047f255326a0d5

mentioned in commit f531e3cec59d40a7173ec572a273b0b718f0b74a

mentioned in commit 9a40d095e876c99c2aa543ab8f5777aca53fd3a2

mentioned in commit 848661856a44de31ee2116c1b5c4445fe6daf654

mentioned in commit 991ddc3d7f67f40e3d210025b6c38726e80ba460

mentioned in commit da8946fcb60cd626dc56011364949d44e5575417

mentioned in commit 109fc49d76913574530ee4da22661dede1c12519

mentioned in commit 14acea31f286541ef75358ee37062ee042ea8ca8

mentioned in commit e4f5ec48928b83ac4c4468386cb341f95b009e93

mentioned in commit bb1dad439a4c9366929a8ef39f693dfc1dbcc678

mentioned in commit 27b62d10180e0a299554fc72d2bd0faacf2e3b75

marked the checklist item A manager has full access to an organization and all teams (only organization model, not like super admin. they will still need to assign themselves permissions) as completed

mentioned in commit c15eca2e581d7f2d5ece59a9895a227fd03716d1

mentioned in commit 488a12df458d21d37ad689213ed1c9aa527fef6e

marked the checklist item A manager can't change their own manager status as completed


Trouble with sourcemaps and babel.

Using gulp-sourcemaps isn't working for me.
gulp.task('build-ts', function() {
	var options = {
		declarationFiles: false,
		noExternalResolve: true,
		noLib: false,
		target: 'ES6',
		typescript: require('typescript')
	};
	return gulp.src(paths.ts)
		.pipe(sourcemaps.init())
		.pipe(tsCompile(options)).js
		.pipe(es6Compile(babelOptions))
		.pipe(sourcemaps.write({ includeContent: true, sourceRoot: '/' + paths.root }))
		.pipe(gulp.dest('output'));
});

I am compiling to ES6 then using babel to compile to ES5.  This is because I am using system module type which isn't supported in TypeScript 1.5-beta and the option isn't accessible through gulp-typescript since it isn't in the enum used for mapping target to module.
The problem is, the above results in the souremap being ES6, not TypeScript.  It is unclear what exactly the problem is, very possibly due to the fact that I have two step transpilation.  Is this a known incompatibility?  If so, is there a workaround?
If I do includeContent the sourcemap ends up pointing at a .js file.  It points at the right path, but the file it needs to be mapped to is .ts, not .js.  If I keep includeContent = true, I can get my source maps in chrome but they are the ES6 output of the tsCompile step, not the TS input to that step (which is what I expected).

Can you try the master from GitHub? I've done some fixes to source maps in the previous week. Also gulp-typescript now supports all targets that TypeScript supports.

Appears to be fixed.  I now get .ts files.


Accessing cognito tokens when they are refreshed

Describe the bug
I'm using Alamofire for API calls in my app, and I'm grabbing the Cognito ID token from the auth session after logging in and adding that to all of my API call headers. This seems to be a supported workflow per the docs, but I can't find a way to reliably update that ID token when it expires and the library refreshes it.
Expected behavior
I'd expect to be able to access some subscription or callback when the tokens are updated.
Environment:

Amplify Framework Version: 1.1.1
Dependency Manager: Cocoapods
Swift Version : 5.0

Hi @conner-christianson
You can listen to auth event using hub and listen to events like 'sessionExpired'
More details are here - https://docs.amplify.aws/lib/auth/auth-events/q/platform/ios

This issue is being automatically closed due to inactivity. If you believe this was closed by mistake, provide an update and re-open it.


Point Cloud Optimization

Hi - I decided to move the issues to a new thread as the last one was mostly just me figuring out how to play with your nodes!
I did manage to improve the quality of the splat by refining some parameters but the point cloud itself is not optimised for marching tetrahedrons - below I show the splat, along with the point cloud and generated mesh (without texture) -

My question is, can we find a way to remove the points which are not contributing to the RGB values? Specifically, can we eliminate the Alpha value points from the cloud?
I also came across and implemented this project, SuGaR  (check this issue) which also suggests a method to extract textures and meshes from Gaussian Splatting which is very efficient (tested on provided and my own datasets) - I do hope you take a look once because it did work very well at extracting meshes from gaussian splats :) - The UV Maps were bad, but still, the quality was decent.

I ran into the same issue - my janky approach to solving was to add a remove background node (https://github.com/Jcd1230/rembg-comfyui-node) after the Zero123 image generation step. Then in the gaussian splatting workflow the image masks will actually be populated and the black background will not contribute to the splats.

Thanks for the effort friend, I am aware of SuGaR, and will definitely take a closer look at their method.
I also adopted a better 3DGS to Mesh Conversion Algorithm in yesterday's update, it use nerf to fit 3DGS and then using marching cube to produce mesh & texture by fitting the nerf, and it works much better & faster, feel free to give a try


Allow for augmentation of text input styles in BB7

I wanted to make a larger input field with stylized text but was unable to since the textInput plugin wipes the css styles.
This change will recognize styles applied by the dev but still allow the BB7 styles to override. The dev, if desired, can then override the BB7 style by including the !important flag.

Seems safe enough
+1

dsosby, have you signed and submitted a signed contributor agreement yet?
You will have to sign an IICL and the company you work for would need to sign a CCLA.  More details on how to contribute can be found here:
http://blackberry.github.com/howToContribute.html


Support building containers with custom backend versions

Summary of Changes

Building the backends (e.g. VAI, ZenDNN and MIGraphX) is moved into the Python script
The Dockerfile generation script adds a hook to pass custom functions
New --custom-backends flag for docker/generate.py to pass a path to a Python script

Motivation
This change adds flexibility with building Docker images using different versions of the backends other than the default one that the server is using.
Implementation
Previously, the process to build or download the backends was hardcoded into either the Dockerfile template or in the generate.py script. The idea here is that now it's all moved to the generate.py script, which has a new flag that lets you pass a path to a Python script. During the Dockerfile generation, the script will check if you passed this flag. If you didn't, it will use the default steps which are the same as before by calling a function. If you did pass a flag, it will look for the same function in the Python file you provided and use its output instead.
The needed functions and variables in your custom script aren't documented yet but you can figure it out by looking at the generate script and seeing what functions/variables it's using from the custom_backends module.
Notes
To enable this, you need to pass --custom_backends /path/to/file to docker/generate.py. If you don't, then nothing changes and the default container is built as usual.

Build successful!