[Feature request] Include dithering

Hi @lautr3k we have a crush on dithering on this thread https://plus.google.com/+ClaudioPrezzi/posts/2aVG4KuaESB?fscid=z13sct1itoz5sthsd22pdfbazufhenlio04.1491053867286977
Could you add https://github.com/noopkat/floyd-steinberg to the filters? It's just a couple of lines:

import floyd_steinberg from 'floyd-steinberg'

...L#133...
settings = Object.assign({}, {
        smoothing   : false,  // Smoothing [true|fale]
        brightness  : 0,      // Image brightness [-255 to +255]
        contrast    : 0,      // Image contrast [-255 to +255]
        gamma       : 0,      // Image gamma correction [0.01 to 7.99]
        grayscale   : 'none', // Graysale algorithm [average, luma, luma-601, luma-709, luma-240, desaturation, decomposition-[min|max], [red|green|blue]-chanel]
        shadesOfGray: 256,    // Number of shades of gray [2-256]
        invertColor : false,  // Invert color...
       dither: false,
}, settings || {})

...L#190...
if (settings.dither) {
        floyd_steinberg(imageData)
}

Or extend lw.canvas-grid to accept custom key callables, this way could be extended without recompiling all the toolchain.

filters: {
   ... your filters
   dithering: (imageData) => { ...process ImageData... }
}

Or a simple hook for further imageData processing


🔗 Define chain specific configs

the first part of #393 ,

refactor chain related config under modal/chain
auto mapping getExplorerAddressLink(address)/getExplorerTransactionLink

to support a new chain: dev could

define a chain profile
import in packages/core/src/constants/chainId.ts
add tests

yarn build/yarn test passed

@tt-marek please give some feedback
though the PR does not change or add new features but propose a new Chain definition format,
PR will get conflict with new chains support like #368

Looks very promising, I'll add comments in the code.
Would be good to rebase on master and add more networks (there was a few PRs with new networks)

❤ ❤ ❤
@gasolin This is very promising PR. It require a bit of work, but it will solve a lot of problems around configuration.
#393 #346 #47
Added comments. Looking forward to merge it.

@tt-marek PR rebased.

rename MULTICALL_ADDRESS to multicallAddress in per chain prop definition
generate default multicall addresses from DEFAULT_SUPPORTED_CHAINS
to scope the impact the refactor work neither expose new functions nor change tests. Can add getNetworkData(chainId) or getMulticallAddressFor(chainId) in future work with test, it will be much easier to do so after landing this PR

updated

@gasolin Great work! ❤ ❤ ❤
If you would like to continue, take a look at #408


Warn people when they're looking at the development version of the docs

Occasionally people find their way to the development version of the tutorial page, and get tripped up when things don't match the current release version:
https://groups.google.com/d/msg/wagtail/HPL5w1LIrAw/SvVLJKgGCgAJ
Django's docs display a notice:

This document is for Django's development version, which can be significantly different from previous releases. For older releases, use the version selector floating in the bottom right corner of this page.

We should see if readthedocs allows us to do the same, and add one if so.

RTD show a notice when viewing an old tag - https://github.com/rtfd/readthedocs.org/blob/499a74076f0debae09acc1d4c4eeeab03f8f557e/media/javascript/rtd.js#L14
I think the same can possibly be done with https://github.com/rtfd/readthedocs.org/issues/152#issuecomment-186115025

After a bit of digging, it looks like a fix to Readthedocs may already be in the works.
Others have run into this issue here: https://github.com/rtfd/readthedocs.org/issues/3481
Also, there is a reasonably recent PR to fix it here: https://github.com/rtfd/readthedocs.org/pull/3547
I will keep an eye on that issue/PR but if others want to +thumbsup on the issue - it might help the RTD team flag that this is important.

I'm not sure if this is new, maybe a Read the Docs option/update?
I noticed today that a small popover appears for a few seconds on the latest docs.

@lb- yep, it's the new readthedocs addons https://about.readthedocs.com/blog/2024/07/addons-by-default/
We enabled the feature when it was still in beta a few months back, but there was an issue with our theme https://github.com/wagtail/sphinx-wagtail-theme/issues/296 so it didn't show up immediately. It's fixed now though, so I think we can close this – thanks!

Thanks @laymonage - this is an awesome win.


ROL-2103 Upgrade vulnerable commons-collections to 3.2.2

https://issues.apache.org/jira/browse/ROL-2103

Looks good to me.


feat: up_conf supports timeout field configuration

e.g:
up_conf = {
        name = upstream_info["name"],
        type = upstream_info["type"],
        nodes = new_nodes,
        timeout = {
            send = upstream_info.timeout["send"],
            read = upstream_info.timeout["read"],
            connect = upstream_info.timeout["connect"]
}

this example is not easy to understand. please provide a real example.


Javascript port

I've create a javascript port of this repo. I'd like to work on that.
The link: nanajs
Currently, I've implemented the lexer of nana.

Everything about nana comes from the proposal.
I have to say that quite a lot of details aren't mentioned. e.g. Does string literal support more complex escaping? In nanajs, I support single-char-escape (\n) and 4-hex (\u0001). Hope you could make it clear later.

There's a new problem. About the gate, will there be a token between symbols?
aba = |param1 param2| (
    param1, param2, param1
);
aba 1 2

Is there any token between param1 and param2 inside ||?

Another problem. Does it support parentheses inside expression?
(
    1, (2+3)*4
)

Will (2+3) be parsed as an tuple and caused an error? Otherwise how to distinguish it?

Hi neko-para!
First I want to appreciate you for your interest on this project. Happy to see you!
I'm sorry that the doc is missing right now. The current proposal you're relying on is more of a cheat sheet than a descent language spec. I'll formalize the concepts in a few days, and you can expect a workable one tomorrow night.
As for the details,

Does string literal support more complex escaping?

Yes. It supports anything the target language (like json) supports. In fact all my lexer does is it contains anything between " and [^\\]", and directly passes it down to the target language.

About the gate, will there be a token between symbols?

Yes. The current gated block syntax is
|x, y| ( x + y )

, which is similar to most language's closure syntax. The , is added because   would lead to the intuition of a function application.

Does it support parentheses inside expression?

Yes. Tuple exists because it binds multiple values into one. So any depth of tuple with single expression inside will be treated as the expression itself.
(((((x+y))))) // equals
x+y

In fact, nana doesn't directly distinguish "an expression wrapped by a pair of parentheses" because unary tuple takes care of it.
Of course, all above are just suggestions. You may give your opinion, and I'll be happy to discuss.
Nice day. (❁´◡`❁)

Also, I think a spec might be hard to read. Maybe a blog style description may serve as a better option?

(a+b)*c
This () don't create a block, does it?
Then how to know whether it should be an () for expr or for tuple? near to a sign(e.g. * / + -)?

At first, I fold the recursive ()
(((a))) every () have only one block child, so I can just fold that. But is there any way to distinguish whether the () is a part of an expression or not?

I've had a solution.
The () for expression is a standalone expression, so it is a value instead of tuple, and we can just perform calculation on it. This may provide a good explaination.

Another problem.
{
    fun = |x, y| (x + 1);
    fun 2 -1
}

There are two types to understand:

fun (2-1)
fun 2 (-1)
How to distinguish this?

I suggest to use certain parenthese to restrict the param of gate
e.g.
{
    fun = |x, y| (x + 1, y - 1);
    fun<1, 2>
}

As currently <> is only used in multi var expose, there won't be ambiguity.

I've had a solution.
The () for expression is a standalone expression, so it is a value instead of tuple, and we can just perform calculation on it. This may provide a good explaination.

Yes, I agree. Tuple is designed to combine multiple values and pass them around, like in python. It makes no sense in making a "tuple" containing only one element. I think your reasoning is correct.
Also I'd like to mention that I plan to use () as unit type. You can treat it as null.

I suggest to use certain parenthese to restrict the param of gate
e.g.
{
    fun = |x, y| (x + 1, y - 1);
    fun<1, 2>
}

As currently <> is only used in multi var expose, there won't be ambiguity.

Well, I don't think that's necessary. One can always write
{
    fun = |(x, y)| (x + 1, y - 1);
    fun (1, 2)
}

This might be confusing if one hasn't touched any functional programming style. Here fun only takes one parameter, which is a tuple, and breaks it apart to (x, y) where x and y are just separate binders.

Another problem.
{
    fun = |x, y| (x + 1);
    fun 2 -1
}

There are two types to understand:

fun (2-1)
fun 2 (-1)
How to distinguish this?

Ouch. Integers are always such a headache, aren't they.
My current plan is to treat -1 as an negative integer, and ...- 1 as an operator. So
fun 2 -1 // ((fun 2) (-1))
fun 2- 1 // fun (2-1)
fun 2 - 1 // fun (2-1)

in my impression that's what most languages' lexers do, but I'm not 100% sure.

Hi.
Just updated perhaps one third of the proposal. The link is here.
I hope that may help.

After reading the new proposal, I'm quite confuse.
(
    factorial x := (
        ? x                         // what is x?
        | 0 -> 1                    // if x is 0
        | _ -> x * factorial (x-1)  // else
    );
    factorial 10
)

As mentioned in it, this is the function. Then what's the difference between gate and function?
In the past, I think that nana only support gate, and the codes above will be things below.
(
    factorial := |x| (
        ? x
        | 0 ->1
        | _ -> x * factorial (x - 1)
    );
)

P.S. As I think the condition is a part of lala, I treat all | as parenthese in nanajs.

The code pieces above are equivalent. Since writing gated blocks all the time could be tiring, the traditional function way of writing it could be easier for new comers to use.

As I think the condition is a part of lala, I treat all | as parenthese in nanajs.

Feel free to do so. We can always write a minimum viable product and scale up based on that.

I rewrite the parser using nearley. Now the grammar won't affect lot.
But here's another question: what syntaxs do condition choice support?
I've seen things like | [x] + xs to pick up the first item inside a vector. But there isn't any explaination about it.
Can I use | xs + [x] to pick the last item? Can I use this to pick tuple? (or use | (x) + xs?) How about | x1 + x2 + x3 + [x] + xn?

Btw, If you consider any language grammar too complicated, you can always remove it for now. I'm good with that.
For example, you may just keep
(
    add := | x, y | ( x + y );
)

as the only legal form.

I've support both now.  It really saves my time😄

Currently the parser works fine with most of grammar(except condition, as mentioned above, I have some question about it).

LOL what a coincidence. We've just commented at the same time.
I'm writing the pattern language part, and it will soon be done. But I think must of you intuition will fit mine, so nothing special.
I think a QQ group could be better at unofficial discussions: 760133331. Feel free to come :-)

I have to say that I cannot find that group.

Interesting.


fix(dependencies): remove deps from the repo

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


java.lang.UnsatisfiedLinkError when setting language

I was able to successfully build the dynamic library using:
./build.py -o libjava-tree-sitter ../tree-sitter-c-sharp-master.
I'm also able to load the library successfully. However, when I try to set the language to C# via parser.setLanguage(Languages.cSharp()), I get a linkage error:
Exception in thread "sbt-bg-threads-1" java.lang.UnsatisfiedLinkError: 'long ai.serenade.treesitter.Languages.cSharp()'
	at ai.serenade.treesitter.Languages.cSharp(Native Method)
	at astute.Main$package$.main(Main.scala:11)
	at astute.main.main(Main.scala:7)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at sbt.Run.invokeMain(Run.scala:144)
	at sbt.Run.execute$1(Run.scala:94)
	at sbt.Run.$anonfun$runWithLoader$5(Run.scala:121)
	at sbt.Run$.executeSuccess(Run.scala:187)
	at sbt.Run.runWithLoader(Run.scala:121)
	at sbt.Defaults$.$anonfun$bgRunTask$6(Defaults.scala:1988)
	at sbt.Defaults$.$anonfun$termWrapper$2(Defaults.scala:1927)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
	at scala.util.Try$.apply(Try.scala:213)
	at sbt.internal.BackgroundThreadPool$BackgroundRunnable.run(DefaultBackgroundJobService.scala:367)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

I was wondering if I could receive some help. And for note, I'm performing the above steps on an M1 Mac and yes, I did take a look at the in-progress PR. It was that PR that helped me successfully build the dynamic library.

Sorry that this response is not an answer to your question.
I think it's worth considering the content here that mentions a few other efforts toward using tree-sitter from Java (including a fork of this repository that might have the issue addressed).

I've tried what you've suggested and now I seem to be getting SIGSEGV when loading in the library:
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00000001046f630c, pid=45986, tid=57199
#
# JRE version: Java(TM) SE Runtime Environment Oracle GraalVM 21.0.2+13.1 (21.0.2+13) (build 21.0.2+13-LTS-jvmci-23.1-b30)
# Java VM: Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21.0.2+13.1 (21.0.2+13-LTS-jvmci-23.1-b30, mixed mode, sharing, tiered, jvmci, jvmci compiler, compressed oops, compressed class ptrs, g1 gc, bsd-aarch64)
# Problematic frame:
# V  [libjvm.dylib+0x54e30c]  jni_GetFieldID+0x148
#
# No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again


Support Multicore changes during Resubmission creation

Fixes #8283

@ticoann basic tests look good and we can merge it for tomorrow testbed.

Thanks, Alan


Added Cypress test for validating MM-18043 Guest User Experience: Verify Guest User Restrictions

Summary
Validating the Restrictions when a Guest User logs in. Please check the ticket for more details.
Ticket Link
Fixes https://mattermost.atlassian.net/browse/MM-18043

@srkgupta Having the same error show up as yesterday, and thus failing.

/cherry-pick release-5.16


Adding post 'Five Advantages of Log-Based CDC'

@gunnarmorling Very nice post! I've added a couple of proposolas as a food for thought but it is ready for publication even now.

Reworked a bit and applied. Thanks for reviewing, @jpechane!


Possible Enhancement: Modification of value to be verified in RuleFor

First off, great library. Thanks for the excellent work here. But I was looking for one feature and I couldn't find it. I think it may be an enhancement that could be useful to many developers.
In my domain, we get a lot of untrimmed strings, prefixed strings and so on. What I want to validate is a transformed value, not the original value. An example follows.
RuleFor(m => m.TransactionId).Transform(t => t.TrimStart("0")).NotEmpty()
It is true that this can be done in custom rules or using Must, but I'm wondering if there is enough interest and use of this pattern to make something like this available. It seems more fluent to me in a way that just using Must.
Interested in feedback and discussion, if enough interest exists, I (or others) can explore how to best add to the library if it makes sense. I totally accept that such as change may not be practical or expedient.

It's a good idea, I'll put it on the backlog for a future release.

Closing issue as resolved. Very much appreciated. While my time for open source projects is sadly limited, I will track this one and contribute if I can.

Re-opening to track development progress.

I've committed this change - it'll be in the 7.5 release.

I've released 7.5 to nuget today that includes this fix.


Loading model files at runtime

Godot version:
N/A
OS/device including version:
N/A
Issue description:
You currently cannot load files such as OBJ, DAE, and GLTF/GLB at runtime.
Steps to reproduce:
N/A
Minimal reproduction project:
N/A
I understand that packing the import plugins with every exported binary isn't great for the many people who don't need this; that's not what I'm asking for. It would be useful to have a compile flag that allowed the import plugins to be compiled into the binary, for those who are willing to compile. Additionally, although I don't know much about how these plugins work, we'd clearly need more than just a compile flag since even when running a game using the editor's binary you still can't easily import files at runtime.
Some members of the community have questioned why you'd ever need this, which is fair enough, so let me give my use case. I'm developing an FPS where maps are downloaded from servers at runtime. Godot scenes can contain scripts & signals which makes them a security risk. Yes, I can strip out scripts programmatically, but I'm not done yet. Needing to import the maps into Godot makes life more difficult for mapmakers, turning away valuable community members from the game. It's much easier for a mapmaker to just click "Export as glTF" in Blender than it is for them to first model it in Blender and then import it into Godot.
Of course, that's just me and my use case. Others don't need this, which is why it should probably be a separate compile flag rather than the default functionality.
Thank you for your time.

Please see https://github.com/godotengine/godot/pull/34193
The api is close to:
func _process(_delta):
   var importer = SceneImporterGLTF.new()
   var root = importer.import_gltf("user://example.glb")

@fire Where is the API Documentation for SceneImporterGLTF?

It's still work in progress and not in 3.2, but I can show you this.

@fire This looks awesome! I am so excited for this.
I will need to import GLTF models at runtime for a project. Would you say that functionality is currently at a usable state in your scene_export_native_gltf branch? Anything I should be aware of when building? I'd be happy to provide feedback if I am able to get this working.

Please try and comment on your problems.
Remember that res:// is read only and user:// is writable.

Thanks for your quick feedback!
Using your suggestion here:

Please see #34193
The api is close to:
   var importer = SceneImporterGLTF.new()
   var root = importer.import_gltf("user://example.glb")

I tried importing my own local gltf file using this Box.glb sample to my user:// dir.
I created an empty scene with a single Base Node class with the following code:
extends Node

func _ready():
	var importer = SceneImporterGLTF.new()
	var root = importer.import_gltf("user://Box.glb")

and I got this error
E 0:00:01.004   import_gltf: Condition ' err != OK ' is true. returned: __null
  <C++ Source>  modules/gltf/editor_scene_importer_gltf.h:74 @ import_gltf()
  <Stack Trace> Testing_Area.gd:13 @ _ready()

Is there anything else you would recommend to help me get this working?

Update your branch and use:
extends Spatial

func _process(_delta):
	if Input.is_action_pressed("ui_accept"):
		var importer = SceneImporterGLTF.new()
		importer.import_gltf(self, "res://door.glb")

At https://github.com/fire/godot/commit/f00f49cf05c39db0d985e4327594bb10d5547ca4
Api changed to:
extends Spatial

func _process(_delta):
	if Input.is_action_pressed("ui_accept"):
		var importer = SceneImporterGLTF.new()
		var node = importer.import_gltf("res://door.glb")
		add_child(node)
		node.set_owner(get_owner())

will this feature be available in 3.2.2?

@PravinKumar95 Nobody has opened a pull request for this feature yet, so no.

I can update my PR to be in 3.2, but I don't think it will be merged in 3.2.
For me, 4.0 currently can't even load gltf2 into meshes.
I've been using the exporter for my projects.
The github master PR pipeline is heavily delayed, so I have been waiting for my Skeleton Inspector PR to be looked. At least until the next time a cool idea shows up.
The only big changes in my testing is I removed CSG import because there's no robust api for getting a mesh out. Will need to revisit.
Another bug is related to when the root node is a mesh in gltf / godot scene. My fixed tried to fix export, but then import broke.

I can update my PR to be in 3.2, but I don't think it will be merged in 3.2.

This will be good enough for me. I only need runtime import of mesh and materials. Thanks

Is there also a way to load compiled scripts at runtime? (C# or third-party bindings)

@Mek101 This is the wrong topic for addressing loading compiled scripts from C# or GDNative.

@PravinKumar95 I've been developing the changes here. https://github.com/godot-extended-libraries/gltf Please file issues if you have problems.
Known issues:

Root nodes with skeletons / mesh instances disappear
CSG support was removed until I get a look at how to get data without modifying the csg class

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.
The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.
If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

I have an module that implements runtime model import. Anyone who is interested can make a proposal and I'll link when I make the pull request.

@fire I see the repo above is archived as it was merged into 4.0, but how difficult would it be to compile it as GDNative for 3.x? Is it something I can do using your archived repo or is the code 4.x specific?

Please use gltf module from https://godotengine.org/asset-library/asset/964
Source is from https://github.com/V-Sekai/godot-gltf-module/tree/gdnative

I'm failing to make it work at runtime. Works perfectly in editor, but SceneImporterGLTF (from comments above) is not recognized and EditorSceneImporterGLTF only works in the editor...

I'm failing to make it work at runtime. Works perfectly in editor, but SceneImporterGLTF (from comments above) is not recognized and EditorSceneImporterGLTF only works in the editor...

Since this is a C++ module, you need to recompile all the export templates you plan to use.

There is a GDNative library included, and I'm not even exporting anything, I'm failing to make it work while running a project in debug from the editor.

@Calinou https://github.com/V-Sekai/godot-gltf-module/tree/gdnative is a GDNative module.
Do you have a sample of your project?
Remember that you manually call PackedSceneGLTF since none of the editor code is in the runtime.

Thanks, the API is what I was missing.
The above didn't work (the class is registered but the method was not found), I actually had to load the GDNative as below (taken from import_vrm.gd):
var flags = 0
var bake_fps = 1000

var gstate = load("res://addons/godot_gltf/GLTFState.gdns").new()
var gltf = load("res://addons/godot_gltf/PackedSceneGLTF.gdns").new()
var instance = gltf.import_gltf_scene(path, flags, bake_fps, gstate)

That worked perfectly for models outside res://

Managed to get the above working thanks @fbcosentino, however when exporting to HTML5 it then stops working, any ideas?

Managed to get the above working thanks @fbcosentino, however when exporting to HTML5 it then stops working, any ideas?

You need to enable GDNative in the HTML5 export preset for GDNative add-ons to work there. The add-on also needs to be compiled for use in HTML5.


Does Cloud Run timeout apply on GKE-Cluster?

Does Cloud Run timeout apply on GKE-Cluster? If it does, will it always apply, or is this being looked in to?

Currently it's 10 minutes. We'll take a look at documenting it as soon as possible. Maybe then I can comment on whether we're planning to change it.

This is documented in the docs, so won't re-document here.
https://cloud.google.com/run/quotas


Highlight matches does not work with color codes

When using color codes to colorize lines, the --highlight-matches argument does not have any effect.
For example, when executing the command
./salomon.sh -a analyze -c log_colors.cfg -i samples/foobar.log -f "2014" --highlight-matches

while the color config file has the following instruction
214         warn

to colorize lines containing the term warn using color code 214, the filter term is not being highlighted. See below.

So far, the matches are now highlighted, but their text is not displayed.

Fixed in version 1.11.1.


modernize output files, refactor testing, add deployment tests for di…

…fferent module formats

Fixes #2719, #2572, #2569, #2559

When is the release happening with the file-saver github changes ? The last release was 1.5 years ago. I will have to move to some other package if it's later than a month.

I can't tell you precisely, although I hope it will be less than a month. Slightly depends on the discussion in #839, where we might introduce additional breaking changes.

I am also really looking forward to the release with the necessary fixes and improvements.
These issues are very critical for us #2820 #2829 and I can see that they have already been fixed in the master branch. As far as I understand, there is a big cumulative update planned here, and its release may be delayed due to discussions in #839 and possible breaking changes.
Will interim updates be released so that users can access the already submitted fixes for known issues? This release will help us a lot.

Good news: 2.0.0 is out!

Thank you @HackbrettXXX , very cool


[SDK-4336] Change default user agent

🔧 Changes
This changes the default user agent slightly to ensure that we can differentiate between traffic from the previous 1.0.0 version of this package and the upcoming 1.0.0
📚 References

🔬 Testing

📝 Checklist

[ ] All new/changed/fixed functionality is covered by tests (or N/A)
[ ] I have added documentation for all new/changed functionality (or N/A)

Codecov Report
Patch and project coverage have no change.

Comparison is base (10e008a) 94.56% compared to head (18ae3da) 94.56%.

Additional details and impacted files
@@           Coverage Diff           @@
##             beta     #233   +/-   ##
=======================================
  Coverage   94.56%   94.56%
=======================================
  Files          44       44
  Lines        8134     8134
=======================================
  Hits         7692     7692
  Misses        345      345
  Partials       97       97

Impacted Files
Coverage Δ

internal/client/client.go
66.47% <ø> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1129)>

Hello,
I get the following error when extracting data from paddlets, what should I do? Thanks.
from paddlets.datasets.repository import get_dataset
ts_data = get_dataset('NAB_TEMP') # label_col: 'label', feature_cols: 'value'

Out:
/Library/Frameworks/Python.framework/Versions/3.9/bin/python3 /Users/x/Documents/tjd/exchange-rate-prediction_2023-05-09/code.py
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 1346, in do_open
    h.request(req.get_method(), req.selector, req.data, headers,
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1253, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1299, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1248, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1008, in _send_output
    self.send(msg)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 948, in send
    self.connect()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/http/client.py", line 1422, in connect
    self.sock = self._context.wrap_socket(self.sock,
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 500, in wrap_socket
    return self.sslsocket_class._create(
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1040, in _create
    self.do_handshake()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ssl.py", line 1309, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1129)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/x/Documents/tjd/exchange-rate-prediction_2023-05-09/code.py", line 31, in <module>
    from paddlets.datasets.repository import get_dataset
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/__init__.py", line 5, in <module>
    from paddlets.pipeline import Pipeline
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/pipeline/__init__.py", line 4, in <module>
    from paddlets.pipeline.pipeline import Pipeline
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/pipeline/pipeline.py", line 16, in <module>
    from paddlets.utils.utils import get_tsdataset_max_len, split_dataset
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/utils/__init__.py", line 6, in <module>
    from paddlets.utils.utils import get_uuid
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/utils/utils.py", line 18, in <module>
    from paddlets.models.data_adapter import DataAdapter
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/models/data_adapter.py", line 4, in <module>
    import paddle
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/__init__.py", line 25, in <module>
    from .framework import monkey_patch_variable
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/framework/__init__.py", line 17, in <module>
    from . import random  # noqa: F401
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/framework/random.py", line 16, in <module>
    import paddle.fluid as fluid
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/__init__.py", line 52, in <module>
    from . import io
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/io.py", line 31, in <module>
    from paddle.fluid import layers
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/layers/__init__.py", line 29, in <module>
    from . import math_op_patch
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/layers/math_op_patch.py", line 24, in <module>
    from paddle.fluid.dygraph.base import in_declarative_mode
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/dygraph/__init__.py", line 35, in <module>
    from . import checkpoint
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/dygraph/checkpoint.py", line 26, in <module>
    from paddle.fluid.dygraph.jit import _SaveLoadConfig
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/dygraph/jit.py", line 34, in <module>
    from paddle.fluid.dygraph.dygraph_to_static import logging_utils
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/dygraph/dygraph_to_static/__init__.py", line 32, in <module>
    from . import convert_call_func
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddle/fluid/dygraph/dygraph_to_static/convert_call_func.py", line 22, in <module>
    import pdb
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pdb.py", line 77, in <module>
    import code
  File "/Users/x/Documents/tjd/exchange-rate-prediction_2023-05-09/code.py", line 33, in <module>
    ts_data = get_dataset('NAB_TEMP') # label_col: 'label', feature_cols: 'value'
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/paddlets/datasets/repository/base.py", line 67, in get_dataset
    df = pd.read_csv(path)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/util/_decorators.py", line 311, in wrapper
    return func(*args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/readers.py", line 586, in read_csv
    return _read(filepath_or_buffer, kwds)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/readers.py", line 482, in _read
    parser = TextFileReader(filepath_or_buffer, **kwds)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/readers.py", line 811, in __init__
    self._engine = self._make_engine(self.engine)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/readers.py", line 1040, in _make_engine
    return mapping[engine](self.f, **self.options)  # type: ignore[call-arg]
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/c_parser_wrapper.py", line 51, in __init__
    self._open_handles(src, kwds)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/parsers/base_parser.py", line 222, in _open_handles
    self.handles = get_handle(
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/common.py", line 609, in get_handle
    ioargs = _get_filepath_or_buffer(
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/common.py", line 312, in _get_filepath_or_buffer
    with urlopen(req_info) as req:
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pandas/io/common.py", line 212, in urlopen
    return urllib.request.urlopen(*args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 214, in urlopen
    return opener.open(url, data, timeout)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 517, in open
    response = self._open(req, data)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 534, in _open
    result = self._call_chain(self.handle_open, protocol, protocol +
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 494, in _call_chain
    result = func(*args)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 1389, in https_open
    return self.do_open(http.client.HTTPSConnection, req,
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/urllib/request.py", line 1349, in do_open
    raise URLError(err)
urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1129)>

using Ubuntu, try the following:
sudo update-ca-certificates --fresh export SSL_CERT_DIR=/etc/ssl/certs


I am able to share (image + text) with whats app in iOS.

Hi,
I need to share image + text in one of my app but i am not able to share both together wit this module in iOS.
I am using latest 1.8.4 version. I can share it separately but am not able to share both.
Please let me know the solution.
Thank you.

        const xhr = Ti.Network.createHTTPClient({
           onload: function () {
              const f = Ti.Filesystem.getFile(Ti.Filesystem.applicationDataDirectory, 'image.jpg');
              if (!f.exists()) f.createFile();
              f.write(this.responseData);
              SocialShareWidget.share({
                 status: 'Check out me on #blablabla',
                 image: f.resolve(),
                 androidDialogTitle: 'Check out me'
              });
           },
           onerror: function (e) {
              console.error('[UI] Could not downloaded image: ' + e.error);
           }
        });
        xhr.open('GET', thumbSrc);
        xhr.send();

Simple example how get temporary image and after sharing that.
I hope it will be useful, for me this approach works on Titanium SDK 7+ (iOS).


Make rescue_from Page not found errors configurable

Since Rails 4 'Exceptions raised inside exception handlers are not propagated up'.
Therefore, ActionController::RoutingError raised in the Comfy::Cms::ContentController cannot be rescued from the application controllers. We can make the call to rescue_from inside ContentController conditional (NOTICE: this workaround won't work if we simply pass if: condition as an option to rescue_from, it just can't be called at all).
Let me know your thoughts and if you agree to go this way I'll prepare some tests.

What a bizzare thing. Add a test for this and let's merge this in.

Any ideas how to refactor this? I don't want to spoil your gem so we can
always treat it as an application concern (and force devs to implement
something like an exceptions_app) or work more on CMS and:

remove rescue_from call from ContentController
refactor find page/raise errors
25 cze 2015 16:41 "Oleg"<EMAIL_ADDRESS>napisał(a):

What a bizzare thing. Add a test for this and let's merge this in.
—
Reply to this email directly or view it on GitHub
https://github.com/comfy/comfortable-mexican-sofa/pull/661#issuecomment-115279206
.

UPDATE: added integration test, updated configuration_test.
I had to modify the ContentController a bit more to make tests running (also, see the workaround in the integration test, setup and teardown). Without this workaround this new feature is not testable because once you run 'rake test' the app is loaded and the ContentController is already parsed (and rescue_from called).
Let me know if you're going to merge this. Alternatively, for someone who doesn't want to use rescue_from 404 provided by the CMS the solution is to unregister the exception handler somewhere in his app with the following:
.rescue_handlers.delete(["ActiveRecord::RecordNotFound", :page_not_found])

FYI: I ended up just using the workaround in the initializer:
+++ b/config/initializers/comfortable_mexican_sofa.rb
 #     return true
 #   end
 # end
+
+# Unset the exception handler for 404
+Comfy::Cms::ContentController.rescue_handlers.delete(["ActiveRecord::RecordNotFound", :page_not_found])

@GBH can this be merged and released?

Content controller is not relying on exception handling to render 404s anymore.


Unable to connect ibeam with paper login IBKR

Hi Voyzan,
Users of IBeam, i would like to know if it possible to use IBeam with paper account with interactive brokers ? thanks
Regards,

Hey @Riyaz84 thanks for outlining your issue 🙂
Yes, it is possible to use IBeam with paper account. You simply need to use your paper account credentials from IBKR, instead of the live trading ones.
See here: https://ibkr.info/node/663

Hi Voyzan,
Thanks for your reply, once after the setup of IBeam on my server, as in attach, i don't have any option to select paper trading account, from the webportal. Thanks.
Regards,

Please get in touch with IBKR support for info on how to get your paper trading account details 👍

Already got one, the question is where to use paper login credentials on the localhost IBeam web portal ?  Thanks

The paper trading account has a unique login, distinct from your live trading account.   So if you use the paper trading account name and associated password, it should log  you into the paper trading account.  At least that's the way I've seen it work.  As a made up example, someone might have a live trading account name of johndoe37 and a paper trading account name of johnva721.

If you log into IB you ought to be able to navigate to the account icon (torso and head icon), then click on Settings.  Under Account Configuration there is a Paper Trading Account link next the Paper Trading Account Number. Click on the Paper Trading Account link.  It should bring up a page detailing the Paper Trading Username.

Thanks for that clarification, it seems there are two different user accounts Live/Demo, and the demo account password need to be reset. Works fine. Thanks


Missing metadata values OIC Provider

Based on some trial-and-error it seems we can use following metadata parameter to the the access token authentication method to client_secret_basic from the default client_secret_post:
<Metadata>
    <!-- ... -->
    <Item Key="token_endpoint_auth_method">client_secret_basic</Item>
    <!-- ... -->
<Metadata>

Could this and other 'hidden' parameters be added to the documentation?

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 8cb586f8-c1ff-7676-08b3-915ee00383b0
Version Independent ID: f1770ad7-7ee4-090b-78f8-85924233b190
Content: Define an OpenId Connect technical profile in a custom policy in Azure Active Directory B2C
Content Source: articles/active-directory-b2c/openid-connect-technical-profile.md
Service: active-directory
Sub-service: b2c
GitHub Login: @mmacy
Microsoft Alias: marsma

@sgdesmet Thanks for your feedback! We will investigate and update as appropriate.

Hey @sgdesmet  I understand the difficulty and issues with trying to implement and work with the technical profiles I'm going to go ahead and forward this information back to our product team. And also see if I can add all the parameters in our docs. Thanks for your feedback, and I apologize for the inconveniences this has caused you.

Hello @FrankHu-MSFT no problem and thank you!

Hey  @sgdesmet
We're looking into the metadata and next month we should have all the production ready parameters for B2C in the document.
Would it be fine to go ahead and close this issue now since there are plans to put this in the docs? Or would you like to leave this issue open?
Thanks,

Frank

Hey @FrankHu-MSFT no problem, closing the issue is fine by me

Not seeing this anywhere in the documentation @FrankHu-MSFT. Any schedule?

Not seeing this anywhere in the documentation @FrankHu-MSFT. Any schedule?


Random strategy moves services constantly

Regression in 1.2.x

This issue is caused by the refactoring in Kontena 1.2 to merge the separate ServiceBalancerJob and GridSchedulerWorker into a single GridSchedulerJob in #1873. Instead of only invoking the GridScheduler rescheduling logic to re-evaluate the scheduler strategy in specific circumstances, it is now run continuously, replacing the separate ServiceBalancerJob checks.
Services using the random strategy would still get shuffled around across nodes on each service deploy with Kontena 1.1, but these service deploys would not be triggered by the continuously running ServiceBalancerJob, as it only compared the number of running service containers, without re-evaluating the scheduler strategy. Certain events would still invoke the the GridScheduler rescheduling logic, causing services using the random strategy to get re-deployed in a similar fashion:

nodes connecting: Agent::NodePlugger
nodes being removed: HostNodes::Remove
on kontena grid update, as part of the #1564 --default-affinity feature: Grids::Update)

With Kontena 1.2, these spurious re-deployments of services using the random strategy now happen continuously, as the continuous GridSchedulerJob now runs the GridScheduler logic, which re-evaluates the service scheduler strategy each time. Running the GridScheduler rescheduling logic continuously assumes that the scheduler strategy is deterministic, and only returns a different set of nodes when new nodes are connected, nodes are removed, or the default affinity filters are changed. This is true for the default ha strategy and the derived daemon strategy. However the random strategy will return different nodes each time it is called, resulting in the random service being shuffled around across nodes constantly. This is more likely to happen the fewer instances of the service there are compared to available host nodes.


Added ability to specify name when uploading using SpatieMediaLibraryFileUpload

Provides ability to specify the name of a file to be stored in the media table. Usage:
SpatieMediaLibraryFileUpload::make('myfiles')
    ->collection('myfiles')
    ->usingName('the_name_of_my_file')

//we can now use:
$post->getFirstMediaUrl('myfiles', 'the_name_of_my_file')

Thanks, I've renamed this to mediaName since that's what Spatie uses internally.

Ah! Just seen that you saw the case of specifying null as default! Sorry I missed that! :)

@danharrin What if I wanted to set the name, using another form input field? How would something like that be possible using the "customProperties" handler?

Probably passing a closure and using $get like normal


Add simnet tests

This PR adds directory which will hold high level integration tests that will run with the simnet set of tools.
At the moment those kind of tests are maintained by the simnet team in simnet repo which is hosted just on gitlab.
This was done in this manner while we developed working version of tools to spawn a simulation network based on a docker image and run tests against this network.
We are still developing simnet and we are thinking how to make it simpler to use.
By moving simnet tests into this repository, the expectations are that developers would be able to maintain and create new tests.
Off course simnet team will help explaining how things work and how to use simnet tools

I don't know I included changes from Seun. I will close this one and open a new one


Ignore CancelledSubscriptionException and AbortedStreamException

.. while handling a WebFlux response.
Motivation:
When handling a HEAD request, Webflux can get the response from a GET
request handler and cancel the body subscription immediately. In that
case, PublisherBasedHttpResponse will fail its completionFuture with
a CancelledSubscriptionException, which will be propagated to WebFlux
as an unhandled exception.
Modifications:

Filter out CancelledSubscriptionException and
AbortedStreamException so that WebFlux does not write an ERROR log
about an unhandled exception.
Do not log at DEBUG level about a cancelled stream when the current
request method is HEAD because it's expected.

Result:

Less noisy error handling
Fixes #1847

Thanks for reviewing!


dart-sass: 1.77.6 -> 1.80.4

Fixes: https://github.com/NixOS/nixpkgs/issues/343290
Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[x] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[x] Tested basic functionality of all binary files (usually in ./result/bin/)
24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

Result of nixpkgs-review pr 350808 run on x86_64-linux 1

14 packages built:

dart-sass
dart-sass.pubcache
gitlab
gitlab-ee
jekyll
jsonnet
jsonnet.doc
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
shopware-cli

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/4734

oh now r-ryantm made a pr https://github.com/NixOS/nixpkgs/pull/352249


Introduce new API to pass the preferred internal crypto provider type to crypto service and crypto util

Description:
Introduce a new API for encryption and decryption operations which provides the internal crypto provider as a method argument. This will help to have multiple encryption/decryption approaches in the product or use for external tools like migration tools
Suggested Labels:

Suggested Assignees:

Affected Product Version:
OS, DB, other environment details and versions:
Steps to reproduce:
Related Issues:

The possible improvement can be found here https://github.com/wso2/product-is/issues/8148


Some SwitchMode improvements

What problem does this PR solve?
Fix #134 and AskTUG-33070.
What is changed and how it works?

During switch mode, if we encounter a store that returns Unimplemented (mainly TiFlash nowadays), simply ignore the error.
Added ./tidb-lightning-ctl --fetch-mode subcommand to print the mode.

Check List
Tests

Unit test
Manual test (add detailed scripts or steps below)

Side effects
Related changes

Need to update the documentation
Need to be included in the release note

Document the new --fetch-mode flag.

take a time to fix ci 😄

This pr just ignores these error logs temporarily.  If ideal, the tikv client should be able to handle the incomplete implementation of these interfaces, right?

LGTM

This pr just ignores these error logs temporarily. If ideal, the tikv client should be able to handle the incomplete implementation of these interfaces, right?

No that's not intended to be temporary. The reason is that failing to switch mode won't cause import to fail (just slower), so ignoring the "Unimplemented" error permanently is fine.
Yes TiFlash should implement the SwitchMode API (by doing nothing at least), which will make the error go truly away.

take a time to fix ci 😄

[2020-03-23T21:43:10.004Z] tests/run.sh: line 30: 17815 Aborted                 (core dumped) bin/tikv-server -C "$TEST_DIR/tikv-config.toml"

We're always enabling TLS and thus hit tikv/tikv#7209 😭. Either we block on tikv/tikv#7251 or disable the TLS temporarily.

/run-all-tests tikv=release-3.1 pd=release-3.1 tidb=release-3.1

/run-all-tests

PTAL @3pointer


Better previews

New design

Pictures now have max-height in vh, so they are bigger/smaller depending on user screen size
Site description is multiline again
Site previews now take all width (see #1084)
Site thumbnail is ~2 times smaller than picture preview

New features

When preview is shown, toggle button is hidden now (only if auto-expand is on):

It's still possible to hide it again using right click:

Now it's possible to expand images from previews:

This works both for pictures and site thumbnails. It's really awesome, believe me!
Bugfixes

Finally fixed these options:

^^ content in backlog is now affected too.

Extra space is removed (mobile only):

TODO

[x] Revert site previews width
[x] Bind "back" button to expanded previews (mobile)
[x] Keep toggle button if auto-expand options are disabled

Forgot to mention that this PR conflicts with #1117

With the toggle hidden, how can one close the embed on mobile?

With the toggle hidden, how can one close the embed on mobile?

The same way - that menu is possible to open on mobile too (via taphold).

:-1: on that aspect of this PR then; should show the toggle for A) continuities sake (you press it to open, makes sense to press it to close) and B) that's not intuitive at all. Other then that I'm mostly supportive of this PR

on that aspect of this PR then; should show the toggle for A) continuities sake (you press it to open, makes sense to press it to close)

Well, we can keep it if auto-expand options are disabled. Because otherwise you almost never press "open" (thanks to p.1 in bugfixes).

I like that the toggle is now shown, takes screenspace (I whole line) when its not really necessary.
generally 👍 looks really good. ill try to merge this into my instance when I get the chance.

On my embed branch, I moved the toggle button where usernames are, this way it's still visible and doesn't take space.

On my embed branch, I moved the toggle button where usernames are, this way it's still visible and doesn't take space.

Yeah, it was my first idea how to do this. But it becomes bad UX-wise, because it is hard guess what this button do (since it placed in different area).
The second idea was to make this IRCCloud-like:

And it's still plan B.
Anyway I think current way (right click) is better, since you can click anywhere on the embed area without any need to search for specific area. I like it, this is a bit more relaxing.

Pictures now have max-height in vh, so they are bigger/smaller depending on user screen size

I'm not sure this is an improvement at all. The whole point of #524, which took months of improvements, reviews and validation, was to make previews more discreet. This PR essentially reverts it.
I have noticed several users had disabled previews because it was too big and they were afraid of NSFW content. This is not a good thing :/

Site description is multiline again

How do you truncate this when it's too big? I haven't looked at the implementation yet, but do you set a max height + ellipsis?

Site previews now take all width (see #1084)

#1084 was not about making them all full width but fixing a bug where link previews were randomly truncated far before the edge of the screen. Sorry my issue title was super confusing, I just reworded for clarity.

Site thumbnail is ~2 times smaller than picture preview

Looking at your screenshots, the very first preview does not look good compared to what we have on master. I believe I have mentioned this earlier, but treating a link preview like this is about a picture is most often wrong. Sure, it's the case when sharing a link to imgur or the like, but on any link that is not about a picture hosting platform, you will not want to display a gigantic picture. Sometimes it shows an avatar, sometimes a logo, etc.
If you want to share a picture, send a link to a picture and the picture preview will do its job.

When preview is shown, toggle button is hidden now:
It's still possible to hide it again using right click:

This is a big no-no to me, for reasons similar to those detailed by @MaxLeiter.

Now it's possible to expand images from previews:
This works both for pictures and site thumbnails. It's really awesome, believe me!

I'm... on the fence for this one. For pictures, definitely 👎. There are no benefits to not opening the image in a new window. If we ever proxy pictures (what @xPaw described here), then there will be a privacy benefit (especially with your link to original picture under the proxied one).
For link preview, there is a benefit, but I'm wondering if the UX is good. What I mean by this is that the full gray box (the link preview) links to the original website, but a small portion of it (the image) now acts differently when you click on it. I understand the goal of what you did, I just don't think that's good UX. As mentioned earlier, the simplest UX (simpler is better in UX) for this is the current solution: if you want to see more of this link, you click on the preview and the page opens. After all, it's a preview, not a view :D

Finally fixed these options:

Sorry, what do you mean by this? (Again, I haven't gone through the code yet, will do)

Extra space is removed (mobile only):

Sweet!

The second idea was to make this IRCCloud-like:

This would work when you open the picture as a popup (after clicking the image), but not on the preview. × carry a meaning of deletion, while this is merely collapsing/hiding.

Anyway, I still need to test it to get the full picture of the new UX, but that's my review of your PR description 😅.

I strongly disagree with you in almost every aspect. I made this changes not because I was bored and had nothing to do :smiley: Previews are really pointless in current state.
This PR makes previews look like in any other modern messenger (discord, telegram, irccloud, ...) - just check them and you'll see that current Lounge system is different.
And now you basically saying that all those people (devs and users of that messengers) are wrong and use system which doesn't look good and has no benefits.

I still need to test it to get the full picture of the new UX

I doubt that your opinion will change, since sites content play main role here.

#1084 was not about making them all full width but fixing a bug where link previews were randomly truncated

Alright, I'll revert this change then.

How do you truncate this when it's too big? I haven't looked at the implementation yet, but do you set a max height + ellipsis?

Ellipsis don't support multiline, so no. I use max height + gradient.

× carry a meaning of deletion, while this is merely collapsing/hiding.

If you have a better idea - share it :smile:

This would work when you open the picture as a popup (after clicking the image), but not on the preview.

Popup is absolutely perfect the way it is in this PR, you'll see, probably >_>

Other things look like a jokes, sorry.

I'm not sure this is an improvement at all.

Really? Maybe we should remove Lounge's responsive design as well? Let's optimize it for my display and call it perfect :laughing:

and they were afraid of NSFW content. This is not a good thing :/

:smile:  How NSFW content is related to this PR? Are you suggesting to make pictures too small so nothing can be seen, or what?

but treating a link preview like this is about a picture is most often wrong

95% of content I see on IRC is perfect with this. Like I said, it depends.

If you want to share a picture, send a link to a picture and the picture preview will do its job.

Yep, I'll ask everyone on IRC to do so.

There are no benefits to not opening the image in a new window.

Yeah... Do you even open pics? Have you noticed broken height/width in half of them? No?

If we ever proxy pictures (what @xPaw described here), then there will be a privacy benefit (especially with your link to original picture under the proxied one).

Actually it's unrelated too, because previews should always lead to proxied images.

if you want to see more of this link, you click on the preview and the page opens.

I don't see anything when preview is too small.

should show the toggle for A) continuities sake (you press it to open, makes sense to press it to close)

Keep toggle button if auto-expand options are disabled - done.

Revert site previews width

Done.

Bind "back" button to expanded previews (mobile)

I decided not doing this, because there are some problems (scroll for example) if code is simple, and it doesn't worth making it more complicated. So I post code here for future reference, if anyone is interested.
if (self.is("img")) {
	window.location.hash = "preview";
	container.html(templates.toggle_expand({link: self.attr("src")}));
} else {
	window.history.back();
}

$(window).on('hashchange', function() {
	var hash = location.hash;
	var preview = $("#expand_preview");
	if ((preview.hasClass("display") && hash.indexOf("preview") === -1) || (!preview.hasClass("display") && hash.indexOf("preview") > -1)) {
		preview.toggleClass("display");
	}
});

So this PR is ready now.

Finally managed to bind back button to this event.

@bews, about this previews, some of the things you noticed as broken has now been ported to master, like:

^^ content in backlog is now affected too.

Extra space is removed (mobile only):

Mind removing this from your PR, and fixing conflicts / rebasing with master?

To be honest, out of this PR, I would only keep the image/thumbnail popups, with a lot of usability/accessibility testing, and maybe the image (not thumbnail) height in vh if we can make it right.
Not multiline descriptions (or only if they can be expanded, and collapsed by default), bigger thumbnails, removing the preview toggle / hiding on right click.
@xPaw, thoughts?

Alright, since we are focusing v2.4.0 around previews, I did test this PR completely this time :)
Let me get back to all of what you listed in your description to be thorough:

Pictures now have max-height in vh, so they are bigger/smaller depending on user screen size

This actually weirder than I thought, so there is definitely some experimenting/tinkering to do and discuss before we can approve this.
⌛ TODO Can you move this into its own PR, please?

Site description is multiline again

:x: This is a definitive no for me. It leads to some very bizarre results (see below for example). Latest master is actually doing a good job about this, so I don't think we'll move forward with this.

Site thumbnail is ~2 times smaller than picture preview

:x: No for me as well. Thumbnails are really good like they are at the moment, and a huge-ass icon doesn't help 😅 And in the end, your image/thumbnail popup solves the issue of not seeing the details of the thumbnails, so that will be perfect.

When preview is shown, toggle button is hidden now (only if auto-expand is on):

:x: This will be obsoleted by #1303, see PR description.

Now it's possible to expand images from previews:

It is pretty cool indeed. Can you move this to its own PR (or remove the different :x:'s from this one) and rebase with master?
There are a few things to improve to be perfect, but that's definitely something we could have for v2.4.0!
⌛ TODO Disable message input focus when the popup is open
⌛ TODO Add a × button on top-right to close the popup. I get that the entire background is clickable to close it, but there needs to be a visual cue
⌛ TODO Resolve conflicts
⌛ TODO (In the future, not for this PR) Left/Right buttons to browse expanded images on the channel would be super sweet

^^ content in backlog is now affected too.

:x: Fixed in master

Extra space is removed (mobile only):

:x: Fixed in master

@bews, is this something you want to help getting into v2.4.0?

This is a definitive no for me. It leads to some very bizarre results (see below for example).

I do not want this title to be cut (which definitely will be issue on mobile), so I'm against this.

No for me as well. Thumbnails are really good like they are at the moment, and a huge-ass icon doesn't help :sweat_smile: And in the end, your image/thumbnail popup solves the issue of not seeing the details of the thumbnails, so that will be perfect.

I strongly disagree with this for the reasons I mentioned several times before. And no, popup doesn't help here, because it's silly to open each image just to see it.

Add a × button on top-right to close the popup. I get that the entire background is clickable to close it, but there needs to be a visual cue

You can click anywhere to close it, not only on background. So this is not needed and will look bad.

Anyway, there's no point to resolve conflicts if we don't agree on this PR, and I have a feeling that we don't :(

Your PR(s) packs up multiple changes/features/fixes. The image viewer has some real potential. I don't think the team will accept the other changes.
The best way to settle this is to split the features so the ones the project owners agree on can make it to master.
Are you willing to do this, or are you set that you will not make changes to the image viewer if we don't accept the other changes you are submitting with this PR?

Your PR(s) packs up multiple changes/features/fixes.

It will be a problem for me to update over9000 branches each time, so I group related thing together.

Are you willing to do this

No, there is no reason to do so. I make PRs just because of 2 things:

To share work with anyone interested (already done by creating PR)
To save time on merging in future

If PR is not merged completely, then I still need to merge remaining parts every time and branch will still exist. Splitting this PR will take some time and make merging harder for me in future, there no gain at all. So I rather keep it in current way and save a lot if time.

I see you have decided to sacrifice a good-looking feature on a project that's trying to improve and on project maintainers who are spending their own free time to review your work. That is rather sad but that's a choice we respect.
Hopefully better luck with future PRs, though I do recommend you look around for tips on open source etiquette, it will make for a better experience for all sides :)
In the meantime, if anyone wants to take over the image viewer, we would be happy to guide through an acceptable feature (unless any @thelounge/maintainers are against this, but I don't believe so).

It looked like you want to talk about my closed PRs, so here we go:

project that's trying to improve

I would argue with this, because each version, little by little you break good things from Shout UI. Sure, there are some good parts too, but they don't worth dealing with broken interface. So I've tried to fix that broken things in my PRs, and got heavy resistance.
For example, in this PR I've tried to fix that small site previews, which are A BIG problem for my screen size and content. There are different ways to fix them: set size in vh, use UI scaling, add option in settings, etc. But you don't even want to discuss it - you ignore this problem and force your way.
This is not how any project should evolve. If you keep breaking UI and ignoring problems that users face with, then this project will be doomed.
Also, you're so eager to :-1: PRs, even here in first comment you said

For pictures, definitely :-1:. There are no benefits to not opening the image in a new window

And now you want it, but code haven't changed at all. It proves that your reviewing is a bit off. Project wiki says:

Project maintainers may also give a :-1: if they have very strong concerns about a PR. Note that a :-1:s are a very drastic measure and thus should be used very sparingly and provided with a thorough explanation and details, either on the PR and/or the IRC channel.

It's definitely not something I see in this PR and my other PRs as well.

maintainers who are spending their own free time to review your work

Why did even wrote this? I definitely spent a lot more time on this PR making it, then you just reviewing. It's your project I try to improve for free in my free time, and you are complaining?  In my projects I'm truly happy when someone tries to help me, so I really can't understand this attitude.

I would argue with this, because each version, little by little you break good things from Shout UI.

No, we change things. You consider them broken because you have a different idea of how the project should progress. That is not a problem, but it does lead to us disagreeing on things and your PRs being rejected.

For example, in this PR I've tried to fix that small site previews, which are A BIG problem for my screen size and content.

Stop using the words "broken" and "fix" for subjective things. You can change these in custom css or fork, and we disagree that it's something that should be in the core project. There is nothing wrong with that.

This is not how any project should evolve. If you keep breaking UI and ignoring problems that users face with, then this project will be doomed.

This is something that has literally come up from 1 person, it is something that you can easily "fix" in your running version with custom css, and you already run a fork.

And now you want it, but code haven't changed at all. It proves that your reviewing is a bit off. Project wiki says:

@astorije didn't want this feature, but having seen that others want it, he has changed his mind. This is literally what you just berated him for not doing. He clearly can't win with you.

You consider them broken because you have a different idea of how the project should progress.

No, I consider them broken because I used this things for a long time and now they don't work. Site previews is one of them: before the changes I was able decide open link or not just by looking on a preview, now I can't.

for subjective things

It's not subjective, if you were in my place, you would want to fix that too. Everything subjective was fixed in my custom theme long ago.

didn't want this feature, but having seen that others want it, he has changed his mind

Where? Where did he saw that? There wasn't any comments except for mine since then.

This is something that has literally come up from 1 person

Lol, you want me to run a voting campaign for bigger site previews or what?

it is something that you can easily "fix" in your running version with custom css, and you already run a fork

Are you suggesting to make local changes instead of creating PR? Otherwise it looks like offtopic.
Also, some changes still need merging, because there is no plugin system. Actually, this can be fixed very easily, but I have a feeling PR with that change will be rejected too.

It's not subjective, if you were in my place, you would want to fix that too

That's the definition of subjective.

Where? Where did he saw that? There wasn't any comments except for mine since then.

In IRC

Lol, you want me to run a voting campaign for bigger site previews or what?

No, I do not, because that's not how things work here. But if it was such a big issue, other people would have mentioned it as well.

Are you suggesting to make local changes instead of creating PR? Otherwise it looks like offtopic.

I'm suggesting that when you have a change that you share with us and we don't want to merge it (which is perfectly normal in an open source project, you cannot expect everything to be merged), that you keep them in your fork. That makes things more awkward for oyu merging things, I understand that, but that cannot be the reason for us accepting things.

Also, some changes still need merging, because there is no plugin system. Actually, this can be fixed very easily, but I have a feeling PR with that change will be rejected too.

1 good way of knowing whether a PR is going to be rejected or not is to discuss with us before implementing something as to whether it's how we want to go about doing things.
You simply cannot expect us to merge all of your PRs. It is clear that we think about the project in different ways, and that's the beauty of open source software.
If you have an idea for how to implement the plugin system, feel free to discuss it with us. I am planning on getting to that point sometime soon, but I don't have a huge amount of time nowadays. #1266 is the first step in plugins, and it is a good start, but it'll take some time.
The key thing is that we don't want to just merge code that "works". We like to think about how these things will change in the future as well. Sure, that makes us slower, and certain things take a long time to end up merging in, but we believe that we end up with better software in the end.
If you think you alone would be better at making an irc client, then feel free to go ahead and make your own. I wish you all the luck, and if it ends up better, I will admit that. But that is not how we work here, you need to accept how we work if you want to help.

If you think you alone would be better at making an irc client, then feel free to go ahead and make your own. I wish you all the luck, and if it ends up better, I will admit that.

You better get ready for this :rofl:


MNU doing changes as user without code modification ... as non-SystemUser (DataCurator)

It looks like RowanProjectService >> changes is doing GsCurrentSession currentSession resolveSymbol: #JadeServer) value., instead of doing an indirect lookup of the JadeServer class.
Here's the stack:
a MessageNotUnderstood occurred (error 2010), a UndefinedObject does not understand  #'value'
---
MessageNotUnderstood >> defaultAction @2 line 3
MessageNotUnderstood (AbstractException) >> _signalWith: @5 line 25
MessageNotUnderstood (AbstractException) >> signal @2 line 47
UndefinedObject (Object) >> doesNotUnderstand: @9 line 10
UndefinedObject (Object) >> _doesNotUnderstand:args:envId:reason: @7 line 12
RowanProjectService >> changes @5 line 5
[] in  ExecBlock1 (JadeServer) >> updateFromSton: @9 line 11
Array (Collection) >> do: @5 line 10
JadeServer64bit32 (JadeServer) >> updateFromSton: @7 line 6
GsNMethod class >> _gsReturnToC @1 line 1

I believe that the proper lookup is
Rowan platform jadeServerClassNamed: #JadeServer.

Fixed in oscar branch ea0af45


Navigating into composite type members

Still investigating into ways to interact with composite types in the postgresql database, we defined a composite that uses a database defined enum. While following this outline we added ModelBuilder.ForNpgsqlHasEnum invocations and further added a plugin that translates composite type members such that the composite type member is mapped in the query. It does so by implementing a custom Expression that basically does this:
// a class that extends Microsoft.EntityFrameworkCore.Query.ExpressionTranslators.IMemberTranslator
public Expression Translate(MemberExpression memberExpression)
{
	if (memberExpression.Member == ValueMember)
	{
		return new GetCompositeTypeMemberExpression(NpgsqlNameTranslator, memberExpression.Expression, memberExpression.Member);
	}

	if (memberExpression.Member == UnitMember)
	{
		return new GetCompositeTypeMemberExpression(NpgsqlNameTranslator, memberExpression.Expression, memberExpression.Member);
	}

	return null;
}

// a class that extends System.Linq.Expressions.Expression
protected override Expression Accept(ExpressionVisitor visitor)
{
	var leftBracket = new CustomUnaryExpression(Instance, "(", Instance.Type, false);
	var rightBracket = new CustomUnaryExpression(leftBracket, ")", Instance.Type, true);
	var memberExpression = new CustomUnaryExpression(rightBracket, $".{MemberName}", Type, true);
	return memberExpression;
}

This works exceptionally well and generates good queries that look alike:
2018-08-14 16:01:27.065 +02:00 [Error] [7856816] [14] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("11"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t"

Unfortunately when adding a simple where clause in the query as follows:
storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == WeightUnit.Tonne)

the following happens:
2018-08-14 16:01:27.065 +02:00 [Error] [7856816] [14] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("11"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE (t.weight).unit = 6"
Npgsql.PostgresException (0x80004005): 42883: operator does not exist: units.weight_unit = integer

When I refactor the where argument out of the query such that it becomes a query parameter the query works as expected:
var weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

2018-08-14 16:13:28.360 +02:00 [Information] [8553120] [17] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executed DbCommand ("10"ms) [Parameters=["@__weightUnit_0='Tonne' (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE (t.weight).unit = @__weightUnit_0"

Is there a way to make this query work without that extra parameter?

I just noticed that changing this:
var weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

to a const variable as the roslynator code analyzer warns about RCS1118:
const WeightUnit weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

causes the query to fail again because the underlying int value of the enumeration is passed into the query as the parameter.

@dpsenner You're running into a few open issues here...
The constant enum issue is similar to the char literal issue. Basically the expression tree stores the underlying value directly (making this issue fairly intractable). There's been some discussion of how to tackle this, such as adding some heuristics to determine when a literal char or enum comparison is happening, but that work is still ongoing.
As for composite mapping, there's some work that needs to happen at the ADO.NET driver before we can add first-class support to the EF Core provider. One recent consideration has been to transparently map composites to ValueTuple.
Based on your snippets, it looks like you're planning to replace some services internal to the EF Core provider. If you're already set on doing this, you should take a look at the separation of responsibilities between our custom expressions, the NpgsqlSqlQueryTranslatingVisitor, and the NpgsqlSqlQueryGenerator.
Right now it looks like you're using CustomUnaryExpression to (creatively) generate SQL for the composite types. While creative, this type of work is much better suited for a service replacing NpgsqlSqlQueryGenerator.

This issue reads as a duplicate of #489. Are those changes up for the next release?
The following is a little bit off topic to this issue, I do however feel that parts of what we are doing could become a part of the postgresql integration with EF core.
We're not considering to replace EF core services or are not seeing that we should do so. Currently the above snippets are part of an NpgsqlEntityFrameworkPlugin that adds database enums and composite types that we have defined in the database. It does so by adding type mappers and IMemberTranslators. It is a spinoff from this conversation.
Feel free to have a look at the plugin. As you see there, we use the NpgsqlEntityFrameworkPlugin system to add support for our types and I assume that this is the initial purpose of this plugin. We have no need to replace ef core internal services. We are somehow abusing the relational type mapping system to inject non-relational composite types. It would be better if there was a way to add composite type mappings in the plugin, also because it would allow us to delegate the member translation of composite types to NpgsqlQuerySqlGenerator which appears to be the right place for this.
The following is the plugin:
public class UnitsPlugin : NpgsqlEntityFrameworkPlugin
{
	public override string Name => "Schema.Units";

	public override string Description => "Provides conversions for unit types and enumerations in the unit schema.";

	protected NpgsqlSnakeCaseNameTranslator NpgsqlSnakeCaseNameTranslator { get; }

	private Dictionary<Type, (RelationalTypeMapping, IMemberTranslator)> CompositeTypes { get; }

	public UnitsPlugin()
	{
		NpgsqlSnakeCaseNameTranslator = new NpgsqlSnakeCaseNameTranslator();
		CompositeTypes = new Dictionary<Type, (RelationalTypeMapping, IMemberTranslator)>();

		AddEnum<UnitKind>();
		AddEnum<AreaUnit>();
		AddCompositeType<Area>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<FrequencyUnit>();
		AddCompositeType<Frequency>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<LengthUnit>();
		AddCompositeType<Length>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<PieceCountUnit>();
		AddCompositeType<PieceCount>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<RatioUnit>();
		AddCompositeType<Ratio>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<TemperatureUnit>();
		AddCompositeType<Temperature>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<VolumeUnit>();
		AddCompositeType<Volume>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<WeightUnit>();
		AddCompositeType<Weight>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");
	}

	public override void AddMappings(NpgsqlTypeMappingSource typeMappingSource)
	{
		base.AddMappings(typeMappingSource);

		foreach ((Type compositeType, (RelationalTypeMapping relationalTypeMapping, IMemberTranslator memberTranslator)) in CompositeTypes.Select(t => (t.Key, t.Value)))
		{
			typeMappingSource.ClrTypeMappings[compositeType] = relationalTypeMapping;
			typeMappingSource.StoreTypeMappings[relationalTypeMapping.StoreType] = new RelationalTypeMapping[] { relationalTypeMapping };
		}
	}

	public override void AddMemberTranslators(NpgsqlCompositeMemberTranslator compositeMemberTranslator)
	{
		base.AddMemberTranslators(compositeMemberTranslator);

		IEnumerable<IMemberTranslator> memberTranslators = CompositeTypes.Select(t => t.Value.Item2);
		compositeMemberTranslator.AddTranslators(memberTranslators);
	}

	private void AddCompositeType<TComposite>(Func<TComposite, string> sqlLiteralFormattingFunc)
		where TComposite : class, new()
	{
		// add this composite type to the underlying npgsql type mapper
		string typeName = NpgsqlSnakeCaseNameTranslator.TranslateTypeName(typeof(TComposite).Name);
		string pgName = $"{Schema.Name}.{typeName}";
		NpgsqlConnection.GlobalTypeMapper.MapComposite<TComposite>(pgName);

		// add EF side type mapper and member translators for this composite type
		var sqlLiteralFormatter = new DefaultSqlLiteralFormatter<TComposite>(sqlLiteralFormattingFunc);
		RelationalTypeMapping relationalTypeMapping = GetRelationalTypeMapping(sqlLiteralFormatter);
		IMemberTranslator memberTranslator = GetMemberTranslator<TComposite>();
		CompositeTypes[typeof(TComposite)] = (relationalTypeMapping, memberTranslator);
	}

	private void AddEnum<TEnum>()
		where TEnum : struct
	{
		// add this enum to the underlying npgsql type mapper
		string enumName = NpgsqlSnakeCaseNameTranslator.TranslateTypeName(typeof(TEnum).Name);
		string pgName = $"{Schema.Name}.{enumName}";
		NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>(pgName);
	}

	private RelationalTypeMapping GetRelationalTypeMapping<TComposite>(ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
	{
		const string schema = Schema.Name;
		string typeName = NpgsqlSnakeCaseNameTranslator.ConvertToSnakeCase(typeof(TComposite).Name);
		string storeType = $"{schema}.{typeName}";
		return new NpgsqlCompositeTypeMapping<TComposite>(storeType, sqlLiteralFormatter);
	}

	private IMemberTranslator GetMemberTranslator<TComposite>()
	{
		return new DefaultCompositeMemberTranslator<TComposite>(NpgsqlSnakeCaseNameTranslator);
	}

	private string ToInvariantString(long value)
	{
		return value.ToString(CultureInfo.InvariantCulture);
	}

	private string ToInvariantString(decimal value)
	{
		return value.ToString(CultureInfo.InvariantCulture);
	}

	private string ToSnakeCase<TUnit>(TUnit unit)
	{
		return NpgsqlSnakeCaseNameTranslator.TranslateMemberName($"{unit}");
	}
}

public class NpgsqlCompositeTypeMapping<TComposite> : NpgsqlTypeMapping
{
	protected ISqlLiteralFormatter<TComposite> SqlLiteralFormatter { get; }

	public NpgsqlCompositeTypeMapping(string storeType, ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
		: base(storeType, typeof(TComposite), NpgsqlTypes.NpgsqlDbType.Unknown)
	{
		SqlLiteralFormatter = sqlLiteralFormatter ?? throw new ArgumentNullException(nameof(sqlLiteralFormatter));
	}

	protected NpgsqlCompositeTypeMapping(RelationalTypeMappingParameters parameters, ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
		: base(parameters, NpgsqlTypes.NpgsqlDbType.Unknown)
	{
		SqlLiteralFormatter = sqlLiteralFormatter ?? throw new ArgumentNullException(nameof(sqlLiteralFormatter));
	}

	public override RelationalTypeMapping Clone(string storeType, int? size)
	{
		return new NpgsqlCompositeTypeMapping<TComposite>(Parameters.WithStoreTypeAndSize(storeType, size), SqlLiteralFormatter);
	}

	public override DbParameter CreateParameter(DbCommand command, string name, object value, bool? nullable = null)
	{
		string sqlLiteral = GetSqlLiteral(value);

		return base.CreateParameter(command, name, sqlLiteral, nullable);
	}

	protected override string GenerateNonNullSqlLiteral(object value)
	{
		return GetSqlLiteral(value);
	}

	private string GetSqlLiteral(object value)
	{
		if (!(value is TComposite composite))
		{
			throw new ArgumentException($"Expected an instance of type {typeof(TComposite)}", nameof(value));
		}

		return SqlLiteralFormatter.GetSqlLiteral(composite);
	}
}

public class DefaultCompositeMemberTranslator<TComposite> : IMemberTranslator
{
	protected Dictionary<MemberInfo, Func<MemberExpression, Expression>> MemberExpressionFactories { get; }

	public DefaultCompositeMemberTranslator(INpgsqlNameTranslator npgsqlNameTranslator)
	{
		MemberExpressionFactories = GetMemberExpressionFactories(npgsqlNameTranslator);
	}

	public Expression Translate(MemberExpression memberExpression)
	{
		MemberInfo member = memberExpression.Member;
		if (!MemberExpressionFactories.ContainsKey(member))
		{
			return null;
		}

		return MemberExpressionFactories[member](memberExpression);
	}

	private Dictionary<MemberInfo, Func<MemberExpression, Expression>> GetMemberExpressionFactories(INpgsqlNameTranslator npgsqlNameTranslator)
	{
		var result = new Dictionary<MemberInfo, Func<MemberExpression, Expression>>();
		foreach (var propertyMember in typeof(TComposite).GetProperties())
		{
			result[propertyMember] = (memberExpression) => new CompositeTypeMemberExpression(memberExpression.Expression, propertyMember.PropertyType, npgsqlNameTranslator.TranslateMemberName(propertyMember.Name));
		}

		return result;
	}
}

public class CompositeTypeMemberExpression : Expression
{
	public override ExpressionType NodeType => ExpressionType.Extension;

	public override Type Type { get; }

	protected Expression Instance { get; }

	protected string MemberName { get; }

	public CompositeTypeMemberExpression(Expression instance, Type memberType, string memberName)
	{
		Type = memberType;
		MemberName = memberName;

		Instance = instance ?? throw new ArgumentNullException(nameof(instance));
	}

	protected override Expression Accept(ExpressionVisitor visitor)
	{
		var leftBracket = new CustomUnaryExpression(Instance, "(", Instance.Type, false);
		var rightBracket = new CustomUnaryExpression(leftBracket, ")", Instance.Type, true);
		var memberExpression = new CustomUnaryExpression(rightBracket, $".{MemberName}", Type, true);
		return memberExpression;
	}

	protected override Expression VisitChildren(ExpressionVisitor visitor)
	{
		return visitor.Visit(Instance);
	}

	public override string ToString()
	{
		return $"({Instance}).{MemberName}";
	}
}

I just added an enumeration member to one of the models that maps to a database enum and it appears like the query generator does indeed map the enum to the enum member:
var meaning = WeighingMeaning.Meaning2;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Meaning == meaning)
	.Select(t => new
	{
		t.Id,
		t.Weight.Value,
		t.Weight.Unit,
	})
	.ToList();

This is what is run against the database:
2018-08-16 12:53:29.780 +02:00 [Debug] [6522008] [16] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=["@__meaning_0='Meaning2' (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = @__meaning_0"

By refactoring the query to this:
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Meaning == WeighingMeaning.Meaning2)
	.Select(t => new
	{
		t.Id,
		t.Weight.Value,
		t.Weight.Unit,
	})
	.ToList();

the following happens:
2018-08-16 12:53:30.307 +02:00 [Error] [6901136] [16] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("12"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"sandbox.weighing_meaning\""
Npgsql.PostgresException (0x80004005): 42704: type "sandbox.weighing_meaning" does not exist
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 444
   at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1130
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)

The query is semantically correct but fails on the syntax because it added quoted around sandbox.weighing_meaning. The query without quotes works fine when run in pgadmin, i.e.:
SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::sandbox.weighing_meaning

It appears that somewhere the integral value is converted back to the enumeration and properly mapped to the correct enumeration member. The fix #490 addresses some issues, but there are still other usecases that do not work as one would expect.

@dpsenner I'll take another look at this tonight, but in the meantime take a look at #554 and let me know if you believe part of your issue overlaps.
Part of this is needing to differentiate between known enum issues, and issues related to composites + your plugin. Any help delineating the two would help immensely.

For one, enumerations that are not added before any dbcontext is created do not work as expected. It is mandatory to add enumerations to the underlying type mapper by invocation of NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>(pgName) right here:
	.UseNpgsql(MrsCoreDatabase.ConnectionString, npgsqlOptions =>
	{
		// adds enumerations and composite types to the underlying npgsqlconnection global type mapper
		npgsqlOptions.AddBackendStorageTypes();
	})

If one does not do this, queries fail with the following symptom:
2018-08-16 17:42:32.790 +02:00 [Error] [6828096] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("34"ms) [Parameters=["@p0='ea8937a0-d0b5-4d91-95e2-c573c0ee8917', @p1='0', @p2='(6,kilogram)' (Nullable = false) (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""INSERT INTO sandbox.weighing (id, meaning, weight)
VALUES (@p0, @p1, @p2);"
Npgsql.PostgresException (0x80004005): 42804: column "meaning" is of type sandbox.weighing_meaning but expression is of type integer

What does not work is adding enumeration types during the model builder phase. To mitigate the issue I just implemented the following extension method which does register npgsql enumeration members but requires the enumeration type to have been registered earlier:
public static ModelBuilder AddEnum<TEnum>(this ModelBuilder modelBuilder, string schema)
	where TEnum : struct
{
	Type enumType = typeof(TEnum);
	INpgsqlNameTranslator nameTranslator = new NpgsqlSnakeCaseNameTranslator();
	string name = nameTranslator.TranslateTypeName(enumType.Name);
	string[] labels = Enum.GetValues(enumType)
		.Cast<TEnum>()
		.Select(t => $"{t}")
		.Select(t => nameTranslator.TranslateMemberName(t))
		.ToArray();

	if (!NpgsqlConnection.GlobalTypeMapper.Mappings.Where(t => t.ClrTypes.Contains(enumType)).Any())
	{
		throw new InvalidOperationException($"The enumeration {enumType} is not registered with the npgsql global type mapper");
	}

	return modelBuilder.ForNpgsqlHasEnum(schema, name, labels);
}

This process feels terrible because there are several places and several times where types have to be registered but knowing this, one can make sure that it works. With enumerations added at the right time, queries fail again with this symptom:
2018-08-16 17:58:02.970 +02:00 [Debug] [7894608] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"public.weighing_meaning\""

Next I just tried to create the enumeration in the public schema:
create schema sandbox;
create type public.weighing_meaning AS ENUM (
	'meaning1',
	'meaning2'
);

create table sandbox.weighing (
	id uuid not null primary key default tooling.create_id(),
	weight units.weight not null,
	meaning public.weighing_meaning not null
);

Doing so and mapping the enumeration along with the schema appears to fail with the same symptom.
2018-08-16 17:58:02.970 +02:00 [Debug] [7894608] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"public.weighing_meaning\""

When I instead register the enum without a schema all my queries and tests pass. I.e. the following works:
NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>("some_enum");

while the following fails:
NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>("public.some_enum");

When I instead register the enum without a schema all my queries and tests pass

Good to hear. This is part of what #554 is looking to fix.

After further investigation I can confirm that we observe exactly two symtoms:

when an enumeration is a member of a relational model and the enumeration is defined in another schema, queries fail because the type is quoted; this is what #554 is about
when an enumeration is a member of a composite type that is added to the global type mapper and not a relational model, the fix implemented with #490 does not work for select queries as previously mentioned in this comment

To me it is still unclear whether this issue can be fixed by improving the plugin. The plugin currently does implement expressions to navigate into composite types members (via IMemberTranslator) and therefore feels like it could be the right place but at the same time it feels to be the wrong place for this. As outlined in a previous comment, it would be ideal if the NpgsqlEntityFrameworkPlugin class provided an api to inject composite types. NpgsqlEntityFrameworkPlugin could then set up composite type members' mappings to work both in select tuples and where queries.


ESP8266 I/O 5V restistant

is the NPN-transistor really needed?
Shouldn't it be possible to connect TX directly to RX without level shifting?
Would make the entire wireing alot easier.

I am not sure if you can run w/o it. Why dont you remove it and test without it?

Searched a bit on that too. And some report it is fine and some say it's speced at 3.3V and might fry the microcontroller but still reported it working with 5V for an extended period. But might decrease lifespan of it.

I have a Wemos mini D1 which I plan to test this on. Can get back to you when i've tested.

Hi,
Normally Rx is open collector. So if you use a pull-up (10k) to 3.3V, you have only 3.3V. The serial signal is inverted, but the newer core software of the ESPs is capable to invert the signal!
Serial.begin(115200, SERIAL_8N1, SERIAL_FULL, 1, true); // true inverts the signal
You find the circuit on:
http://weigu.lu/microcontroller/smartyReader_P1/index.html

Hello,
Were you successful with removing the NPN?

I tried with an ESP32, it worked fine with inverting there instead of using the NPN. I tried using the internal pullup resistor but I don’t think it’s large enough so I added a 10k resistor to 3.3V. I did have to use additional capacitors (2x10uF) to get the ESP32 to power correctly.
If the ESP8266 can power fine without additional caps, I think all you need is a 10k resistor.

Does this mean that i can connect D1 like this:
5V and RTS to 5V.
Data GND and GND to GND.
TX to RX and invert the pin in code.
In other words exclude the voltage divider and NPN-transistor?

No transistor nor divider needed. Pullup resistor and a capacitor (if the energy meter doesn’t supply enough current)

Thank you Fredrik!

For those who want to do the same to invert the pin in software just change uart to below. Might need to change GPIO ports if not on NodeMCU ESP8266.
uart:
id: uart_bus
tx_pin:
number: GPIO1
inverted: true
rx_pin:
number: GPIO3
inverted: true
baud_rate: 115200


Feature/epe 148

In reference to: https://blockone.atlassian.net/browse/EPE-148
First attempt at fixing the eof of cfile

this was setup wrong, closing