Make explicit configuration choices override those set earlier in load order

Right on the heels of both implementing (#156) and further clarifying the behavior (#160), I wonder whether the implemented behavior is worth violating the Principle of Least Surprise.
If I'm using an application and I specify a command-line flag I will expect that it applies the requested behavior unless I happen to know (or see it clearly mentioned in Help output) that my specifying a flag will fail to have an effect if I choose a value that is the default for the application.
To be clear, picture this configuration load scenario:

A default value of text is set for the log format
Environment variables are evaluated/scanned and one for this setting is not found (no change)
Config file is loaded and json value for the log format is found and set
Command-line flags are evaluated and text was specified as the value for the log format

Net result? json wins out because the incoming value for the command-line flag is a default value and the prior value is not.

This would prove surprising to me and likely to a user as well. I think it is fine to feather the configuration settings as the README (now) shows, but any explicit settings specified by later (re load order) config options should override those set earlier, regardless of whether the incoming value is a default or not.

Perhaps generate the base config object using default values, but generate the config file and flag (really all other) config objects using invalid/placeholder values. If the placeholder values are found, disregard them (probably explicitly note this using the LogBuffer object) and keep the prior value, otherwise use the new value.

Relevant:

https://github.com/spf13/cobra/issues/23
https://github.com/spf13/cobra/issues/453#issuecomment-303548563

Opened issue here: https://github.com/alexflint/go-arg/issues/94
Light research indicates that I might be able to pull this off using custom parsing:
https://github.com/alexflint/go-arg#custom-parsing

Implement encoding.TextUnmarshaler to define your own parsing logic.

Perhaps if pelletier/go-toml provides the same custom parsing option I can use custom struct types in place of the mostly primitive types to hold the intended value and alongside it a boolean flag noting whether it was set?

https://godoc.org/github.com/spf13/pflag#FlagSet.Changed
https://github.com/spf13/pflag/blob/master/flag.go#L508

// Changed returns true if the flag was explicitly set during Parse() and false
// otherwise
func (f *FlagSet) Changed(name string) bool {
	flag := f.Lookup(name)
	// If a flag doesn't exist, it wasn't changed....
	if flag == nil {
		return false
	}
	return flag.Changed
}

https://github.com/kubernetes/kops/pull/2765#discussion_r143681405

Nit: It's probably better to use the Changed field on the flag (doing this in the cobra.Command Run function instead) (spf13/cobra#23). The reason being that then the -1 default value won't appear in the help!

I noticed this in the latest help output for this app (I was keying off of -1 to indicate a field that had not be updated by a config file, env var or command-line flag). It stuck out rather prominently.

https://godoc.org/github.com/pelletier/go-toml#ex-package--Tree
This demos using fetching the data directly instead of calling toml.Unmarshal():
config, err := toml.LoadFile("config.toml")

if err != nil {
    fmt.Println("Error ", err.Error())
} else {
    // retrieve data directly
    directUser := config.Get("postgres.user").(string)
    directPassword := config.Get("postgres.password").(string)
    fmt.Println("User is", directUser, " and password is", directPassword)

    // or using an intermediate object
    configTree := config.Get("postgres").(*toml.Tree)
    user := configTree.Get("user").(string)
    password := configTree.Get("password").(string)
    fmt.Println("User is", user, " and password is", password)

    // show where elements are in the file
    fmt.Printf("User position: %v\n", configTree.GetPosition("user"))
    fmt.Printf("Password position: %v\n", configTree.GetPosition("password"))
}

The package also has a GetDefault method that could be used to return an "obvious" placeholder value that our MergeConfig function could match on to determine whether the incoming setting should override an earlier value.
https://godoc.org/github.com/pelletier/go-toml#Tree.GetDefault

When processing env vars, you're early enough in the process that you don't have to worry with merging: you assign what you find.
When processing config files, merging in non-default, valid values should be "good enough".
When processing command-line flags, I'll need some way to know what options were explicitly set. Ideally the command-line flag package would have a way of turning off or disabling pulling in config values from other sources (e.g., alexflint/go-arg package will automatically pull in env var values). Thanks to Alex Flint for pointing that out; I was so focused on detecting explicit flags that I forgot how that package would build its own unified set of config options based on flags and env vars.
If I can detect that flags were explicitly set, I think I can pull this off without too much trouble.
If alexflint/go-arg does not or will not (e.g., the desired behavior is out of the scope for the package) support this, I will need to (re)-evaluate some of the packages which do support the behavior. Out of those available, I suspect that targeting a config struct is unsupported.
Some potential candidates for use:

https://github.com/DavidGamba/go-getoptions
https://github.com/spf13/viper

this handles env vars, config files and flags

https://github.com/spf13/pflag

https://godoc.org/github.com/spf13/pflag#Parsed
https://godoc.org/github.com/spf13/pflag#Flag

Another option that I've not fully thought through yet:
Have alexflint/go-arg target a separate struct with the majority of fields of either string or []string type. I could then have the default values set to something like PLACEHOLDER and if present, the option hasn't been explicitly set, if not, then run validity checks against the provided (and converted) value before merging into a target config struct.

What if the precedence order was changed?
https://github.com/urfave/cli/blob/master/docs/v1/manual.md#precedence
This notes that their package places environment variables higher in the list than config file values:

The precedence for flag value sources is as follows (highest to lowest):
Command line flag value from user
Environment variable (if specified)
Configuration file (if specified)
Default defined on the flag

Maybe I assumed the convention was config file over env variable and that's not the case? If we change the precedence order, does that make config merging an easier problem to solve?

After following some of the ideas found in https://github.com/google/go-github I updated the Config struct fields to be private and created Getter methods (using former CamelCase public field names per the Effective Go doc.
Unfortunately it was after I finished the work and squelched the linting/compile errors that I saw this output:
config\config.go:71:2: `logFilePath` is unused (structcheck)
        logFilePath   *string `toml:"log_file_path" arg:"--log-file,env:ELBOW_LOG_FILE" help:"Optional log file used to hold logged messages. If set, log messages are not displayed on the console."`
        ^
config\config.go:72:2: `consoleOutput` is unused (structcheck)
        consoleOutput *string `toml:"console_output" arg:"--console-output,env:ELBOW_CONSOLE_OUTPUT" help:"Specify how log messages are logged to the console."`
        ^
config\config.go:73:2: `useSyslog` is unused (structcheck)
        useSyslog     *bool   `toml:"use_syslog" arg:"--use-syslog,env:ELBOW_USE_SYSLOG" help:"Log messages to syslog in addition to other outputs. Not supported on Windows."`
        ^
config\config.go:69:2: `logLevel` is unused (structcheck)
        logLevel      *string `toml:"log_level" arg:"--log-level,env:ELBOW_LOG_LEVEL" help:"Maximum log level at which messages will be logged. Log messages below this threshold will be discarded."`
        ^
config\config.go:70:2: `logFormat` is unused (structcheck)
        logFormat     *string `toml:"log_format" arg:"--log-format,env:ELBOW_LOG_FORMAT" help:"Log formatter used by logging package."`
        ^
config\config.go:62:2: `paths` is unused (structcheck)
        paths           *[]string `toml:"paths" arg:"--paths,env:ELBOW_PATHS" help:"List of comma or space-separated paths to process."`
        ^
config\config.go:63:2: `recursiveSearch` is unused (structcheck)
        recursiveSearch *bool     `toml:"recursive_search" arg:"--recurse,env:ELBOW_RECURSE" help:"Perform recursive search into subdirectories per provided path."`
        ^
config\config.go:41:2: `appName` is unused (structcheck)
        appName        *string `toml:"app_name" arg:"-"`
        ^
config\config.go:42:2: `appDescription` is unused (structcheck)
        appDescription *string `toml:"app_description" arg:"-"`
        ^
config\config.go:43:2: `appVersion` is unused (structcheck)
        appVersion     *string `toml:"app_version" arg:"-"`
        ^
config\config.go:44:2: `appURL` is unused (structcheck)
        appURL         *string `toml:"app_url" arg:"-"`
        ^
config\config.go:54:2: `keepOldest` is unused (structcheck)
        keepOldest     *bool     `toml:"keep_oldest" arg:"--keep-old,env:ELBOW_KEEP_OLD" help:"Keep oldest files instead of newer per provided path."`
        ^
config\config.go:55:2: `remove` is unused (structcheck)
        remove         *bool     `toml:"remove" arg:"--remove,env:ELBOW_REMOVE" help:"Remove matched files per provided path."`
        ^
config\config.go:56:2: `ignoreErrors` is unused (structcheck)
        ignoreErrors   *bool     `toml:"ignore_errors" arg:"--ignore-errors,env:ELBOW_IGNORE_ERRORS" help:"Ignore errors encountered during file removal."`
        ^
config\config.go:50:2: `filePattern` is unused (structcheck)
        filePattern    *string   `toml:"pattern" arg:"--pattern,env:ELBOW_FILE_PATTERN" help:"Substring pattern to compare filenames against. Wildcards are not supported."`
        ^
config\config.go:51:2: `fileExtensions` is unused (structcheck)
        fileExtensions *[]string `toml:"file_extensions" arg:"--extensions,env:ELBOW_EXTENSIONS" help:"Limit search to specified file extensions. Specify as space separated list to match multiple required extensions."`
        ^
config\config.go:52:2: `fileAge` is unused (structcheck)
        fileAge        *int      `toml:"file_age" arg:"--age,env:ELBOW_FILE_AGE" help:"Limit search to files that are the specified number of days old or older."`
        ^
config\config.go:53:2: `numFilesToKeep` is unused (structcheck)
        numFilesToKeep *int      `toml:"files_to_keep" arg:"--keep,env:ELBOW_KEEP" help:"Keep specified number of matching files per provided path."`

Yep. Unfortunately I forgot that the go-toml package (the stdlib json package too I think) require public fields in order to populate stucts. Looks like that may be why the google/go-github package uses the Get prefix for its Getter methods?
Taking another look, I see json struct tags sprinkled liberally about their structs, so that's probably the case.
Example:
// apps.go

// App represents a GitHub App.
type App struct {
	ID          *int64     `json:"id,omitempty"`
	NodeID      *string    `json:"node_id,omitempty"`
	Owner       *User      `json:"owner,omitempty"`
	Name        *string    `json:"name,omitempty"`
	Description *string    `json:"description,omitempty"`
	ExternalURL *string    `json:"external_url,omitempty"`
	HTMLURL     *string    `json:"html_url,omitempty"`
	CreatedAt   *Timestamp `json:"created_at,omitempty"`
	UpdatedAt   *Timestamp `json:"updated_at,omitempty"`
}

// github-accessors.go

// GetID returns the ID field if it's non-nil, zero value otherwise.
func (a *App) GetID() int64 {
	if a == nil || a.ID == nil {
		return 0
	}
	return *a.ID
}

It seems that I don't know how to properly initialize/assign values to struct fields of pointer type. More research/testing.

It seems that I don't know how to properly initialize/assign values to struct fields of pointer type. More research/testing.

https://stackoverflow.com/questions/43811652/golang-assigning-a-value-to-struct-member-that-is-a-pointer

Received this via Twitter discussion:
https://play.golang.com/p/NYoOD_rN03p
Need to go back over it (only briefly scanned it) and study when time permits.

Latest stab at this: 46e85c28c30cbc9d22585b9bfb3c6ef49b5159c7
$ go build
$ ./elbow
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x584727]

goroutine 1 [running]:
github.com/atc0005/elbow/config.Config.Version(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /mnt/t/github/elbow/config/config.go:394 +0x37
github.com/alexflint/go-arg.NewParser(0x0, 0x0, 0xc0000d1e28, 0x1, 0x1, 0xc0000d1bd0, 0x40dbc8, 0x30)
        /home/ubuntu/go/pkg/mod/github.com/alexflint/go-arg@v1.2.0/parse.go:217 +0x98a
github.com/alexflint/go-arg.MustParse(0xc0000d1e28, 0x1, 0x1, 0x0)
        /home/ubuntu/go/pkg/mod/github.com/alexflint/go-arg@v1.2.0/parse.go:79 +0x54
github.com/atc0005/elbow/config.NewConfig(0x5f3eb4, 0x5, 0x5ffe8b, 0x70, 0x5faffd, 0x20, 0x5f3fe0, 0x5, 0x0)
        /mnt/t/github/elbow/config/config.go:167 +0x570
main.main()
        /mnt/t/github/elbow/main.go:50 +0xa3

Digging further and was reminded that as part of generating Help output, the Description() and Version methods were being called. Those methods were attempting to dereference pointers that (at that point in the code) had no value. I replaced the bare dereference attempts with Getter method calls and that solved the immediate issue.
I'll need to dig further to see how I can partially initialize all config structs to have a useful value. Perhaps move the initial default values for app name, url, version, etc to the Getter methods?

Is something like this horribly un-idiomatic?
config.SetNumFilesToKeep(config.GetNumFilesToKeep())

Current thinking is that the Getter method can return default values instead of manually setting them inside of the NewConfig() function. This would allow non-configured structs to have starter values for methods that are called on them (provided those methods don't access struct fields directly), but should the config struct already be configured via flags, env vars, config file then those values would be returned.

At this point I've mostly fleshed out a direction I think I'm going to go for a time. I worked out most of the invalid pointer dereference issues, but when running a battery of tests (external, far from complete or "proper") I got output that I tracked down to an error from the logrus package:
Failed to write to log, invalid argument
Failed to write to log, invalid argument
Failed to write to log, invalid argument
Failed to write to log, invalid argument

After additional digging, I found that I made at least two mistakes:

failed to properly panic/abort after merging in config settings from the go-arg package and finding that the result failed validation (e.g., providing a known invalid flag value)
failed to properly guard against an unexpected flag setting by specifying a safe default case statement

I don't know which is more idiomatic:

providing a safe default case statement even if I don't expect it will ever be used
trigger a panic if the default case statement is used (tempting, very, very tempting)
keep switch options limited to just valid values and "be careful"

I suspect this is not the recommended way to handle this

As an example, here is the function containing the switch block that I mentioned:
func SetLoggerConsoleOutput(logger *logrus.Logger, consoleOutput string) {

	var loggerOutput *os.File

	switch {
	case consoleOutput == "stdout":
		loggerOutput = os.Stdout
	case consoleOutput == "stderr":
		loggerOutput = os.Stderr
	}

	logger.SetOutput(loggerOutput)

}

Until I learn differently, I'm going to add a default case statement here that calls panic. Once I can confirm best practice I can go further from there.

From a commit message I just pushed:

Need to refine; should NewConfig() handle panicing or should this
behavior be handled by main() function?
In short, would I expect the go-arg, go-toml and other config
packages to immediately panic when problems are encountered
or would I expect them to return the error "to me" in main() so
I could consider further for final behavior?
I think the latter ...

I'm already relying upon the immediate behavior noted here:

https://godoc.org/github.com/alexflint/go-arg#MustParse
MustParse processes command line arguments and exits upon failure

Though to elaborate, this is if an invalid flag is passed, not an invalid/unsupported setting for the flag. A close approximation is if the specified config filename doesn't exist. That should also result in an immediate failure?

I'm going to take a few more passes to make sure I didn't miss something obvious, but I think I've gotten past the major blockers. At this point I think I'm ready to start carefully confirming that config merging and precedence is working, then work on overall cleanup a little more.
At that point (a few days of very broken, off/on work) I think I'll have something ready to merge into master for an eventual x.y.Z bugfix release.
Out of this we'll need to update the precedence listed in the README to reflect what is really taking place:

Default values
Config file
Environment variables
Command-line flags

The last config source wins. The README has the order incorrect, placing environment variables as lower precedence than the config file.
Around that time I'll also need to do some research to determine if there is a best practice around that precedence. If there is, I'll need to file a separate issue for how best to go about updating the code to respect it. As things stand now, there isn't a way to force a config file to have higher precedence than environment variables since the alexflint/go-arg package sees environment variables and command-line flags only, not allowing for another package to insert itself in the precedence order.

Note to self:
A failure in the configuration processing path results in a nil configuration returned to main(). At that point an entirely default config struct (behind the scenes) is used to generate a help message to the console.
Said another way, even if someone chose to override the application metadata via config file it wouldn't be applied at this point; the built-in values for app metadata are used when invalid configuration settings are encountered.

https://stackoverflow.com/a/45189630/903870
Might be worth rethinking the current setup.

Don't use getters and setters if your fields are exported. This just confuses the interface.
If you need a getter or setter, because it does something (validation, formatting, etc), or because you need the struct to satisfy an interface, then don't export the underlying field!
If you need an exported field for the sake of JSON, database access, etc, and you need a getter/setter, then use two structs, one exported and one private, and define a custom JSON marshaler (or db access method) on the public one:
type jsonFoo struct {
    Owner string `json:"owner"`
}

type Foo struct {
    owner string
}

func (f *Foo) SetOwner(username string) {
    // validate/format username
    f.owner = username
}

func (f *Foo) Owner() string {
    return f.owner
}

func (f *Foo) MarshalJSON() ([]byte, error) {
    return json.Marshal(jsonFoo{
        Owner: f.owner,
    })
}

How that works with pointer fields, I don't know yet. I'll need to take another look at the google/go-github package to confirm I'm following the same basic approach that they are, minus my choice to use the getter methods as the way of obtaining default application values vs theirs to only return zero-values.

https://stackoverflow.com/a/45189630/903870
Might be worth rethinking the current setup.

Perhaps as a later revision to clean things up. I'm starting to feel "branch fatigue" at this point, so I'm not sure I want to radically change things up again so soon. Instead, I think I should focus on cleaning what I've done thus far up, continue to test and get ready to merge the changes in.

As I'm cleaning out of various tabs from prior research I found this thread:
https://forum.golangbridge.org/t/write-logs-to-memory/4521
They provide a sample usage of the logrus package and a bytes.Buffer:
package main

import (
	"bytes"
	"fmt"

	log "github.com/sirupsen/logrus"
)

func main() {
	memLog := &bytes.Buffer{}
	log.SetOutput(memLog)

	log.WithFields(log.Fields{
		"animal": "walrus",
		"number": 1,
		"size":   10,
	}).Info("A walrus appears")

	fmt.Println(memLog)
}

This is a great deal simpler than what I came up with and I suspect it likely has other benefits that I'm not thinking of. It may be worth spinning off going with that approach (vs keeping the current logging.LogBuffer implementation) as a separate issue.

This is a great deal simpler than what I came up with and I suspect it likely has other benefits that I'm not thinking of. It may be worth spinning off going with that approach (vs keeping the current logging.LogBuffer implementation) as a separate issue.

The one thing I am not sure that approach handles that the LogBuffer does is allow for delayed filtering of log messages. Using the bytes.Buffer approach, the threshold has already been applied, either keeping or tossing based on that existing threshold. Need to confirm.

I had planned to handle this in the next milestone, but I've dug into the Go testing package/framework and am taking baby steps towards proving that the latest changes work as intended. It's going to take some time to get everything covered, but so far it's at least making sense.

Breadcrumb for tests:
Type switch.
switch i := x.(type) {}

Could this make testing via table tests easier for validations like LogLevel?

Some notes to myself to record my current line of thinking.
https://stackoverflow.com/questions/24534072/how-to-compare-if-two-structs-slices-or-maps-are-equal
Working up scratch notes for building MergeConfig() tests. Instead of trying to build table tests against each possible option across three different potential configuration sources, I'm leaning towards building either several individual tests in a staged pattern like so:

base + file
base + file + env
base + file + env + flags

or building one longer test that evaluates the current state of the baseConfig struct.
In both scenarios a comparison would be made against the current state of the baseConfig configuration object that other sources are merged into, and a config struct that represents the expected state of the baseConfig struct.

I can either use the "golden" struct for comparison approach or compare each field individually. If individually there may be away to use a test table that I've not thought of yet.
If comparing structs (what is wanted vs what actually happened), this package looks useful:
https://github.com/google/go-cmp
Seeing that is is coming from Google, it should be a solid package to use for testing purposes.

If comparing structs (what is wanted vs what actually happened), this package looks useful:

https://github.com/google/go-cmp
https://godoc.org/github.com/google/go-cmp/cmp#Equal

Seeing that is is coming from Google, it should be a solid package to use for testing purposes.

https://www.geeksforgeeks.org/structure-equality-in-golang/
The earlier upsides acknowledged, there are also benefits from sticking with the standard library and using reflect.DeepEqual(). While it is described as "slow", the focus here is on testing, not use in a hot code path for a production environment.

Ran into another problem, opened an issue in the alexflint/go-arg project repo. In short, the go test framework is passing in command-line flag options which the alexflint/go-arg package is interpreting as invalid flags for it and I don't know how to configure it to ignore the test framework flags.
The notes from the official testing documentation about using a TestMain(t *testing.T) function don't seem to work, but perhaps I'm just doing it wrong. I'm hoping that the package author will get back to me and point out how I'm doing just that.
I'm currently planning to postpone further efforts to implement a TestMergeConfig() function and instead craft my own non-test framework version of the function as a small app to itself that can be run as needed to verify that the merge functionality still works as expected. Far from ideal, but I could theoretically revisit the work later once I learn more about Go's testing support and implement the test code properly.

The package author responded and thankfully it was an easy fix: set os.Args to nil before calling arg.MustParse().
In my case I followed advice mentioned elsewhere to capture the old value and then restore it when finished, but overall the fix is as indicated. The TestMergeConfig() function is working as intended!

As noted yesterday, it seems that the blockers have been removed and so far; I am hopeful that the logic bugs present in earlier versions have been resolved, based on what the tests are showng.

One big item that needs to be fixed is the precedence details. The reality is that the current selection of packages doesn't allow for environment variables to be overridden by config file entries, so my earlier plan of having precedence work in that order will need to be discarded for now; I'll need to update the docs to fix the precedence details.
I also still need to do the research to learn which of the two should override the other. The Twelve-Factor App methodology discourages the use of config files and for the most part I'm coming to agree with the idea, but I still feel there are a large number of one-off or command-line applications that would benefit from being able to reference a common set of configuration options, such as what a config file would provide. That said, I don't know that there are a lot of use cases for needing to override environment variables with a config file. If anything, setting an environment variable would probably be an effort to override a setting elsewhere?
refs:

https://12factor.net/config
https://en.wikipedia.org/wiki/Twelve-Factor_App_methodology

Note to self:
The current TestMergeConfig function assumes that we can safely compare  the newest config struct against the baseConfig object to prove that the merge was successful.
We need to be able to also test and ensure that partial configurations merge properly. To do this, we may need to construct an "expected" config struct and compare that against the baseConfig object. Come to think of it, I think that is what I had in mind to start with. The TestMergeConfig function (along with the Compare function), was meant to work with a base-case scenario, believing that if it worked, so would the complete merge process.

We need to be able to also test and ensure that partial configurations merge properly. To do this, we may need to construct an "expected" config struct and compare that against the baseConfig object. Come to think of it, I think that is what I had in mind to start with. The TestMergeConfig function (along with the Compare function), was meant to work with a base-case scenario, believing that if it worked, so would the complete merge process.

Implemented and working.

At this point I think I've got "enough" to work with for crafting a new PR to finish the work started in #156. Before I do, I think I'll go ahead and remove the support for overriding the application metadata:

name
version
url
description

I can't come up with a good reason why this is needed. If I was building a framework for multiple applications perhaps, but not for this.
Once that is done I'll likely create an "archival" branch for later reference that doesn't have the commits squashed, and then refresh the current branch and squash down to just one.

Question to self:
Where to move/place the metadata details? They're needed early in the application setup process; perhaps just leave them as provided by the Getter methods, but remove support for overriding via config file? That should cover most of the bases for now.

As part of the cleanup I've opted to change the AppMetadata fields from pointer type back to value types. This requires a lot of adjustments across tests and validation code. I've taken a hatchet and have made bulk changes, but still have a ways to go.
When finished, I'll need to make sure that I've not brute-forced the AppMetadata fields and that the merge behavior is still being tested.

When finished, I'll need to make sure that I've not brute-forced the AppMetadata fields and that the merge behavior is still being tested.

Just spot checked the TestMergeConfigUsingIncompletConfigObjects() function and it looks like I opted to use the baseConfig values for the AppMetadata fields for the other config objects (putting the focus on merging other fields), so nothing lost there.


gvim for Windows: setting file contents encoding to UTF-8/16 makes filename incorrect

Describe the bug
In gvim for Windows, setting set file contents encoding to UTF-8 or UTF-16 makes file name incorrect, if the name contains e.g. Cyrillic letters.
To Reproduce
Create a file with Cyrillic letters in name or path (e.g. "мойфайл.txt"). Open the file in Notepad, enter some Cyrillic letters (e.g. "текст", save the file and close Notepad.
Open the file with gvim. See that the letters are displayed incorrectly ("С‚РµРєСЃС‚", i.e. UTF characters displayed in some 8-bit encoding, maybe CP1251), but the file name is displayed OK ("мойфайл.txt").
To fix the issue with text, enter the command "set encoding=utf-16", or "set encoding=utf-8" (same effect). See that the text is now displayed correctly, but file name is not ("...", character codes instead of Cyrillic letters). Modify the text and save the file. See that the file is saved under incorrect name ("ìîéôàéë.txt" or like that, looks like CP1251 translated to US-ASCII).
Expected behavior
Both text and filename are correct. I.e. changing text encoding does NOT change encoding of the file name.
It is necessary to edit "regular" text files under Windows, which have both UTF contents and UTF names.
Environment:

Vim 8.12 or other 8.x
OS: Windows 10 Pro Russian x64, 18363 (looks like any Windows with UTF for text files and file names)

You should add set encoding=utf-8 in your .vimrc (or _vimrc), and also set fileencodings properly.

As Ken wrote, if you change 'encoding' while Vim already has some text in memory, including a file name, it may become invalid.  Vim cannot convert all text it has when you change 'encoding'.  Either change 'encoding' before doing anything, or after changing 'encoding' re-edit files and don't use buffer contents.


Bump IREE version to 3.0.0rc20241118

Bumps the IREE version (which is among other things used to build shortfin) to the latest release candidate / nightly version.

FYI @MaheshRavishankar.


add ramp

give the ability to progressively increase the avoid_kmers_boost weight. If after a certain number of tries (or the boost gets too high), default to the idt codon optimizer

the boosting was implemented in 71097b9be0a238273486124a54606aa1a400b29d. The Defaulting to idt codon optimizer wasn't and probably wont' be added


How do I access chart specific ticks formatter from the label callback?

Say I have a chart:
  new Chart(foo, {
    type: 'bar',
    options: {
      scales: {
        y: {
          ticks: {
            callback: function (val) {
              return val + '%';
            },
          },
        },
      },
    },
    data: {
      labels: [...],
      datasets: [...],
    },
  });

This formats the value of the y axis label, however doesn't affect the context.formattedValue value specified in a Chart.defaults.plugins.tooltip.callbacks.label callback. Previously I used this._chart.options.scales.yAxes[x].ticks.callback to access it from the callback. However, this callback doesn't seem to be available in the label callback anymore. Am I missing something? Thanks so much for your hard work on this library!

You are missing the scales configuration change. We don't have yAxes anymore.
    plugins: {
      tooltip: {
        callbacks: {
          label(ctx) {
            const scale = ctx.chart.scales.y;
            return scale.options.ticks.callback.apply(scale, [ctx.parsed.y, 0, []]);
          }
        }
      },

Off topic: Linear scale formats its "label" using  options.locale


fix zooming image can drag totally outside of the screen

Details
Fixed Issues

$ https://github.com/Expensify/App/issues/16558
$ https://github.com/Expensify/App/issues/16558#issuecomment-1491082951
Tests

android and ios

Open mobile App and go to any chat
Take a screenshot image (or any image with large dimensions) and send it in chat.
Click to view in image previewer
Zoom in the image and drag/swipe it.
Verify that image can't be dragged totally outside of screen.

[x] Verify that no errors appear in the JS console

Offline tests

N/A
QA Steps
Same as Tests step

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://user-images.githubusercontent.com/41129870/232909108-74cf4318-b29d-4794-81da-bebc7e427bcb.mov

Mobile Web - Chrome
https://user-images.githubusercontent.com/41129870/232909825-4ba0d71e-665a-40e3-b305-08835732f6df.mov

Mobile Web - Safari
https://user-images.githubusercontent.com/41129870/232909515-56b250ce-d90c-436f-8c19-7cc34872fc46.mov

Desktop
https://user-images.githubusercontent.com/41129870/232908887-611820f3-e58d-41d3-b900-08f0c3d988f9.mov

iOS
https://user-images.githubusercontent.com/41129870/232871764-263b482c-cd41-4324-9eab-3aef35c06f6c.mov

Android
https://user-images.githubusercontent.com/41129870/232871788-9d6bfa5a-5534-4aa4-bc55-b8bcad518339.mov

@youssef-lr Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@0xmiroslav add videos to all platforms, but zoom feature not work on mobile web chrome and safari, the issue found before this fix, it found now on mobile web https://staging.new.expensify.com.
I will report it in Slack channel if not found in reported issues.

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to staging by https://github.com/techievivek in version: 1.3.4-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.4-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


unknown flag --app-version

Describe the bug
Running bugsnag-cli create-build without app-version says "missing app version"
Providing app version says "unknown flag --app-version"
The two errors are contradictory and confusing
Steps to reproduce

Run bugsnag-cli create-build --api-key=1 --app-version=1
See error

Environment

bugsnag-cli version: 2.2.0
Platform: MacOS

Example code snippet
Run these commands on terminal
(base) ➜  ~ bugsnag-cli --version
2.2.0
(base) ➜  ~ bugsnag-cli create-build
[ERROR] missing api key, please specify using `--api-key`
(base) ➜  ~ bugsnag-cli create-build --api-key=1
[ERROR] missing app version, please specify using `--app-version`
(base) ➜  ~ bugsnag-cli create-build --api-key=1 --app-version=1
bugsnag-cli: error: unknown flag --app-version

Error messages:
bugsnag-cli: error: unknown flag --app-version

Hi @bytehala
Thank you for raising this with us.
As of v2.0.0 of the bugsnag-cli, the --app-version flag has been renamed to --version-name in order to remain consistent with naming conventions, along with a few other flags: https://github.com/bugsnag/bugsnag-cli/blob/v2.0.0/UPGRADING.md
It appears that our documentation and the message that the CLI displays when an app version isn't included hasn't been updated accordingly, so we have now added a task to change this at the earliest convenience.
If you attempt to create a build using --version-name instead, you should find that this works. Please let us know if you have any further issues with this.

Hi again @bytehala
It turns out that the incorrect error message in the bugsnag-cli was fixed in the v2.3.0 release: https://github.com/bugsnag/bugsnag-cli/releases/tag/v2.3.0
We have also now updated our documentation to reflect these flag name changes: https://docs.bugsnag.com/build-integrations/bugsnag-cli/create-build/#available-options
As such, I am now closing this issue. Please feel free to reach out again if you have any other issues, either via GitHub, or by opening a ticket with us directly by contacting<EMAIL_ADDRESS>


Trying to enter vim command shrinks the screen

im on Mac OSX 10.13.3
oni2 0.5.1-nightly
Trying to enter vim command, so pressing :, shrinks the workable screen area. Dragging the screen will reset it.

There's a couple other actions that will randomly do this as well, like switch files.

What a strange issue! Thanks for logging it and including the gif @shan98 - very helpful.
I tested on OSX 10.14/10.15 - I suspect this may be specific to 10.13.
Just curious - when you say other actions cause this, like switching files - how are you switching files? Is it via Command+P, clicking in the explorer?
Would it be possible to get a trace log, reproducing the issue?
You can get one by running this in the terminal: /Applications/Onivim2.app/Contents/MacOS/Oni2_editor -f --trace --log-file=trace.log
I suspect there is a bug in libvim that is causing us to reset our scaling - the trace log might help narrow it down and show some information about the scaling / display state that could be relevant.

Sure thing!
I was switching files by clicking on the tabs, but it was kinda inconsistent in triggering it. But as soon as i hit Command+p, it triggers the shrink every time.
trace.log

Thanks for the log @shan98 !
I see a few places where something is triggering the window to re-measure metrics (framebuffer):
[TRACE]    +0ms Revery.Core.Window : updateMetrics - new metrics: { scaledSize = { width = 1278; height = 778 };
  unscaledSize = { width = 1278; height = 778 };
  framebufferSize = { width = 1278; height = 778 }; devicePixelRatio = 1.;
  scaleFactor = 1.; zoom = 1.; isDirty = false }

... I suspect this is related. We're missing logging to show why we're updating metrics, so I'm going to add that. Also adding some logging around the actual rendering (scale, zoom, etc - curious if those are changing between the renders).
I'm wondering if this could be related to differences in handling the NSHighResolutionCapable or NSSupportsAutomaticGraphicsSwitching on 101.3 - still investigating

Okay thanks a lot for looking into this!


CXR-CLIP code is not found

I was unable to find the relevant code through given link in paper "CXR-CLIP: Toward Large Scale Chest X-ray Language-Image Pre-training" and your repository list. I badly want the code, where can I find it?

I'm not the author of the paper, but I asked my colleague who is. The code hasn't been released yet, but it's expected to be available within a month. For more details, you can contact<EMAIL_ADDRESS>

I'm not the author of the paper, but I asked my colleague who is. The code hasn't been released yet, but it's expected to be available within a month. For more details, you can contact<EMAIL_ADDRESS>
Thanks!


*: remove opp_neg

From rust 1.7, overflowing_neg is supported, so we don't need opp_neg anymore.

LGTM.

ping

/run-integration-test


Switch to using JSONPath from client-go

Right now we have our own implementation of JSONPath which is quite simple, and does not support iterators of lists, etc.
We can leverage the JSONPath library from client-go:
https://github.com/kubernetes/client-go/tree/master/util/jsonpath
This would provide support for more complex queries that includes lists, etc.

Current implementation
https://github.com/keikoproj/cluster-validator/blob/6a52e6df726b93db385ca0a6668948a14b11050d/pkg/api/v1alpha1/resource.go#L42-L53
Field Validation:
https://github.com/keikoproj/cluster-validator/blob/6a52e6df726b93db385ca0a6668948a14b11050d/pkg/client/validator.go#L241-L280

Since we will be accepting jsonpath as a more complex statement seems will be easier to take values separately, e.g. instead of
    fields:
    - path: "{.containerStatuses[*].state.running.startedAt=*"
    - path: "{.status.phase}=succeeded,running"

It would probably be easier to values to the struct
    fields:
    - path: "{.containerStatuses[*].state.running.startedAt" # default to "*" when values not provided

    - path: "{.status.phase}"
      values:
      - succeeded
      - running


Make stuff work

Hopefully soon

Pull Request Test Coverage Report for Build 2672

2 of 3 (66.67%)  changed or added relevant lines in 2 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.06%) to 68.651%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

util/validation.js
1
2
50.0%

Files with Coverage Reduction
New Missed Lines
%

util/validation.js
1
67.52%

model/entity.js
2
63.16%

Totals

Change from base Build 2671:
0.06%

Covered Lines:
7428

Relevant Lines:
10186

💛  - Coveralls


Issue labels

This document details the labels used in this issue tracker
Type labels
The following labels identify the type of the issue and are mutually exclusive:

bug
cleanup
note
question

Feature requests do not have a specific label, they are the default.
Additional labels

blocks 1.0: issue must be resolved before 1.0 is released.
maybe: there is not yet enough evidence to determine if the issue is a good idea. Eventually, either the issue turns out to be a good idea (and has this label this removed), or the issue turns out to be a bad idea (and gets closed).

So I've given up on the effort of categorizing Elvish issues using the "Project/Component/Area" system - it's impossible for me to keep everything in mind, and the benefits they provide are doubtful. I am removing them, and updating the original comment of this issue.

My opinion is that for an open source project at this stage of its evolution there shouldn't be much more than a "bug" label to make it easy to identify bugs that need to be resolved and possibly a "question" label.
A "question" label is useful in a couple of ways. First, a "question" type issue should have the highest priority. Higher than an issue with a "bug" label. Second, as people continue using the issue tracker for reporting bugs, desired enhancements, questions, and other issue types people can filter on "label==question" to get a sense for what sorts of problems people are experiencing using the project.
Labels other than the above two tend to very subjective in my experience and are hard to justify for a project at this stage of its life-cycle.

Since the labels are now simple enough and hopefully self explanatory, I'll close and unpin this.


Failed to enable the wgrep support of urgrep through the straight's use-package integration.

I try to install urgrep with straight and using its wgrep support as follows, but all failed:
1.
(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep")
  :config
  (load "urgrep-wgrep.el" nil t t)
  )

and
(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep")
  :config
  (require 'urgrep-wgrep)
  )

The above two configurations will trigger the following Warnings:
Error (use-package): urgrep/:config: Cannot open load file: No such file or directory, wgrep Disable showing Disable logging

(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep")
  :config (use-package urgrep-wgrep))

The above configuration will trigger the following Warnings:
 Error (use-package): urgrep/:config: Could not find package urgrep-wgrep. Updating recipe repositories: (org-elpa melpa gnu-elpa-mirror el-get emacsmirror-mirror) with ‘straight-pull-recipe-repositories’ may fix this Disable showing Disable logging
See here for the related discussion. Any hints for this problem?
Regards,
HZ

See the :includes keyword in the documentation:
https://github.com/raxod502/straight.el/tree/develop#the-recipe-format

(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep"
		   :includes urgrep-wgrep
		   )

  :config
  (use-package urgrep-wgrep)
  )

Will trigger the following:
Error (use-package): urgrep-wgrep/:catch: Cannot open load file: No such file or directory, wgrep Disable showing Disable logging
(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep"
		   :includes urgrep-wgrep
		   )

  :config
  (require 'urgrep-wgrep)
  )

Will trigger the following:
Error (use-package): urgrep/:config: Cannot open load file: No such file or directory, wgrep Disable showing Disable logging

No such file or directory, wgrep indicates that there is a (require 'wgrep) somewhere in the author's package. My hunch is they do not declare wgrep as an explicit requirement (because the main package doesn't depend on it). Straight cannot detect the dependency in this case, so you would have to install it yourself before requiring that library.

Your hunch is correct!
The problem can be solved with either of the following configurations:
(use-package wgrep)

(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep"
		   :includes urgrep-wgrep
		   )

  :config
  (use-package urgrep-wgrep)
  )

Or
(use-package wgrep)
(use-package urgrep
  :straight (:host github :repo "jimporter/urgrep"
		   :includes urgrep-wgrep
		   )

  :config
  (require 'urgrep-wgrep)
  )


Please help.

I have this error. "ICE failed, your TURN server appears to be broken, see about:webrtc for more details
Peer connection is closed between you &  1fw5ely6fboiksg83tj
Object {  }
state: failed RTCMultiConnection.min.js:18:29996
Peer connection is closed between you &  1fw5ely6fboiksg83tj
Object {  }
state: closed RTCMultiConnection.min.js:18:29996
Peer connection is closed between you &  1fw5ely6fboiksg83tj
Object {  }
state: closed"
How to fix this?

webrtcweb.com is down and coTURN and all other services (including apache2) are stopped. In the meanwhile, you can remove all links to coTURN:
Use following code in your HTML file:
connection.iceServers = [{
        'urls': [
            'turn:webrtcweb.com:4455?transport=udp', // restund udp
        ],
        'username': 'muazkh',
        'credential': 'muazkh'
    },
    {
        'urls': [
            'stun:stun.l.google.com:19302',
            'stun:stun.l.google.com:19302?transport=udp',
        ]
    }
];

If possible, please link & use third party TURN servers.

I (accidentally) tried to upgrade ubuntu and it affected everything including apt tool. I asked my service-providers to format and install older version of ubuntu, again.
Because apt tool is not working, so I can not do anything myself.

Sir it it possible to run a stun and turn server localy? And sir I have
email on you.
On Wed, Oct 10, 2018, 12:11 PM Muaz Khan<EMAIL_ADDRESS>wrote:

webrtcweb.com is down and coTURN and all other services (including apache2)
are stopped. In the meanwhile, you can remove all links to coTURN:
Use following code in your HTML file:
connection.iceServers = [{
'urls': [
'turn:webrtcweb.com:4455?transport=udp', // restund udp
],
'username': 'muazkh',
'credential': 'muazkh'
},
{
'urls': [
'stun:stun.l.google.com:19302',
'stun:stun.l.google.com:19302?transport=udp',
]
}
];
If possible, please link & use third party TURN servers.
I (accidentally) tried to upgrade ubuntu and it affected everything
including apt tool. I asked my service-providers to format and install
older version of ubuntu, again.
Because apt tool is not working, so I can not do anything myself.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/muaz-khan/RTCMultiConnection/issues/649#issuecomment-428431597,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AnlPnB7ImaEq76rQFl27IitjxhRfWn-Xks5ujXNtgaJpZM4XUh1D
.

I've fix coturn today. It should work now.

Yes sir it is working now. But I want to try to run to my own local turn and stun server how to do that?  I already downloaded the simple-turn-windows-386.exe and try to run it but it only opens CMD in 1 second then suddenly close. What could I do for this?

Sir do you really know how to use this as my stun and turn server? Link: http://www.stunprotocol.org/

Okay sir. Now how can I used this? Do i need to change the codes on the IceServersHandler.js file? How?

https://www.rtcmulticonnection.org/docs/iceServers/

// first step, ignore default STUN+TURN servers
connection.iceServers = [];

// second step, set STUN url
connection.iceServers.push({
    urls: 'stun:yourSTUN.com:port'
});

// last step, set TURN url (recommended)
connection.iceServers.push({
    urls: 'turn:yourTURN.com:port',
    credential: 'password',
    username: 'username'
});

Okay sir I'm gonna check this out tommorow gonna have a rest for today.
Anw sir do you know something that turn server on node.js? Because I already have on STUN (THe one you gave on the comment)

Hello Mr. Muaz, I too have same issue
On firefox console i found this error :
I have this error. "ICE failed, your TURN server appears to be broken, see about:webrtc for more details
Peer connection is closed between you & 1fw5ely6fboiksg83tj
I tried the above solutions but none of them worked for me.
I am using RTCMultiConnection v3.4.4, also tried 3.6.9.
Issue is when i connect from the same network, streamer and viewer end works fine, while if streamer & viewer are on different network then viewer screen gets blank.
FYI : I am using "kurento media server" and nodejs.


Substitution doesn't work if the Key has a character from the start or end values

The following test will fail:
        [Fact]
        public void Should_get_substituted_value_when_using_long_substituable_pattern()
        {
            var configurationBuilder = new ConfigurationBuilder()
                .AddInMemoryCollection(new Dictionary<string, string>()
                {
                { "Foo", "Hello %(env,Testo)%" },
                { "Testo", "world" }
                })
                .EnableSubstitutions("%(env,", ")%");

            var configuration = configurationBuilder.Build();

            // Act
            var substituted = configuration["Foo"];

            substituted.Should().Be("Hello world");
        }

I was able to resolve it by changing the Regex to this:
_findSubstitutions = new Regex(@"(?<=" + escapedStart + @")((?!" + escapedStart + ").)((?!" + escapedEnd +").)*(?=" + escapedEnd + @")",
                RegexOptions.Compiled);

I am not an expert with regex, so I don't know if this is the best way.  It seems to allow all of the existing tests to pass.

@EnderBlue sorry for the delay, I'll try to look into it this evening!

@molinch any updates on this?

Fixed with https://github.com/molinch/ConfigurationSubstitutor/commit/58282dd3524d87ea3c62c0e343a321999da4e382
Published nuget 1.1.2
https://www.nuget.org/packages/ConfigurationSubstitutor/


Race condition between page fetch and item list update causing item duplication

Hi,
This issue seems to be a race condition that occurs when a new page is being fetched (via addPageRequestListener) and the item list is being updated (via UpdateFavorite).
Here's a brief overview of the situation:
`
pagingController.addPageRequestListener((page) async {
add(GetRestaurants(page));
});
on((event, emit) {
final itemList = pagingController.itemList;
if (itemList == null || itemList.isEmpty) return;
ListRestaurantDM restaurants = ListRestaurantDM(restaurants: itemList);
restaurants = restaurants.updateFavorite(
restaurantId: event.restaurantId,
isFavorite: event.isFavorite,
);
//When updated itemList and the pagingControllerState is "ongoing" the addPageRequestListener listens again for the same page.
pagingController.itemList = restaurants.restaurants;
});
`
The problem occurs when GetRestaurants is called to fetch a new page (which means the state is ongoing), and UpdateFavorite is called concurrently to update the itemList of the pagingController. This results in the same page being called multiple times, leading to item duplication in the list.
This issue only occurs when a page fetch is in progress and the item list is updated simultaneously.
Do you have any recommendations on how to resolve this, or is this something that needs to be addressed within the package itself?
Thank you for your time and assistance.

Widget _buildListItemWidget(
BuildContext context,
int index,
List itemList,
) {
if (!_hasRequestedNextPage) {
final newPageRequestTriggerIndex =
max(0, _itemCount - _invisibleItemsThreshold);
  final isBuildingTriggerIndexItem = index == newPageRequestTriggerIndex;

  if (_hasNextPage && isBuildingTriggerIndexItem) {
    // Schedules the request for the end of this frame.
    WidgetsBinding.instance.addPostFrameCallback((_) {
      _pagingController.notifyPageRequestListeners(_nextKey!);
    });
    _hasRequestedNextPage = true;
  }
}

final item = itemList[index];
return _builderDelegate.itemBuilder(context, item, index);

}
How to prevent the method from calling notifyPageRequestListeners when the paging is ongoing?
the notifyPageRequestListeners is already called (in progress) while building the widget.

Hi, were you able to resolve this?

I'm also facing the same issue.

Please create gist so that we can replicate the issue to fix it.

I have a page with multiple pagingControllers, each fetching data for it's respective tab, and each with infinite items. I was able to workaround this issue by adding a debouncer to each of the pagination methods passed to addPageRequestListener. I just needed to add a short delay and pass the same unique tag to all the pagination methods, so that no two requests can ever be triggered simultaneously. This ultimately resolved the race condition, and so the duplication for me. Code looks like so:
_tabPagingController.addPageRequestListener((pageKey) {
      debounce(
        'uniqueTag',
        const Duration(milliseconds: 500),
        () => _paginate(
          pageKey: pageKey,
          pagingController: _tabPagingController,
          futureToRun: futureToRunForSpecificTab(currentPage: pageKey),
        ),
      );
    });

We need a working example. @faisalansari0367 asked but no response was given from the reporters. Closing the issue for now, but feel free to reopen.

Hi, I sincerely apologize for the delay in my response. @faisalansari0367 @EdsonBueno
Issue Description
While fetching the next page, if the user updates an item's state (for example, changing an item's 'favorite' status), the list state is updated through pagingController.itemList = updatedItems;. This also triggers another request for the same page.
https://github.com/EdsonBueno/infinite_scroll_pagination/assets/91136431/30134c55-6053-4a1a-a961-dcaf338f007b
Note: I added a delay to my request to simulate scenarios, which does not mean that the problem rarely occurs. This has happened to me so many times, that even with the QA testing team, I had to copy and modify the package to implement a quick fix.
My Solution
The ongoing state is not enough. I added a new state called fetchingNextPage. This state will be used to indicate whether the client is in the process of fetching the next page. While the state is fetchingNextPage, any event that triggers a next page fetch will be ignored. This will prevent multiple requests for the same page number in the scenarios you described.
Before:

After:

I have a page with multiple pagingControllers, each fetching data for it's respective tab, and each with infinite items. I was able to workaround this issue by adding a debouncer to each of the pagination methods passed to addPageRequestListener. I just needed to add a short delay and pass the same unique tag to all the pagination methods, so that no two requests can ever be triggered simultaneously. This ultimately resolved the race condition, and so the duplication for me. Code looks like so:
_tabPagingController.addPageRequestListener((pageKey) {
      debounce(
        'uniqueTag',
        const Duration(milliseconds: 500),
        () => _paginate(
          pageKey: pageKey,
          pagingController: _tabPagingController,
          futureToRun: futureToRunForSpecificTab(currentPage: pageKey),
        ),
      );
    });

it works on my case.
here is my scenario: I have 3 tabs on infinite scrolling items. each has a refresh indicator. onRefresh it will call pagingController.refresh. however every time i switch back to other page and come back to previous page and refresh it will fetch repeatedly the same page. example:
tab1 has 1 page of 2 items.

go to tab2
go back to tab1
refresh tab1
tab1 now has 4 items. 2 of which are the same.
go to tab2 again
go back to tab1
refresh tab1
tab1 now has 6 items. 3 of them are the same items

idk when did the pagingController called another notifyPageRequestListener after I already called pagingController.refresh. also why does it happen to tabbar? can anyone explain to me here.


event.stopPropagation does not work for a modal('show')

Here is my fiddle : https://jsfiddle.net/3drpkwpk/14/

                Just a test <button type="button" id="firstBtn">One</button>
                <button type="button" id="secondBtn">alert One</button>

                                Test One

$('li').click(function() {
    alert('li');
});

$('#firstBtn').click(function(event) {
    event.stopPropagation();
    $('.comment_One').modal('show');
});

$('#secondBtn').click(function(event) {
    alert('secondBtn');
    event.stopPropagation();
});

If I click on the li, I have the alert "li", if I click on the second button, I have only the alert "secondBtn" but if I click on the first one, then, my modal shows, and when it disappears, the alert "li" is called !
Detected on http://stackoverflow.com/q/28896427/535203 and removed angular.

Hi @anthony-o!
You appear to have posted a live example (https://fiddle.jshell.net/3drpkwpk/13/show/light/), which is always a good first step. However, according to Bootlint, your example has some Bootstrap usage errors, which might potentially be causing your issue:

line 40, column 29: W007: Found one or more <button>s missing a type attribute.
line 40, column 63: W007: Found one or more <button>s missing a type attribute.
line 15, column 7: W013: Bootstrap version might be outdated. Latest version is at least 3.3.2 ; saw what appears to be usage of Bootstrap 3.3.1
line 27, column 7: W013: Bootstrap version might be outdated. Latest version is at least 3.3.2 ; saw what appears to be usage of Bootstrap 3.3.1

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!
(Please note that this is a fully automated comment.)

Fixed all the issues, and still have the bug: https://jsfiddle.net/3drpkwpk/14/

The stopPropagation works fine and prevents the click event from the button as expected. What you're seeing is a separate click event entirely. (I would suggest you try logging the targets of the events next time.)
For example, click on the modal body itself after the modal is shown and note that "li" is alerted because you've put the modal as a descendant of the
. You're just seeing the same thing with the modal backdrop.
However, #15881 will move the backdrop element to be a child of the  in Bootstrap v3.3.4.
Note that we don't recommend cramming a modal under an
 like this in the first place:

Modal markup placement
Always try to place a modal's HTML code in a top-level position in your document to avoid other components affecting the modal's appearance and/or functionality.

You're right, I didn't realized that, if I move the modal it's ok: https://jsfiddle.net/3drpkwpk/15/


Add development instructions

What does this PR do?
add dev instructions
Where should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
What are the relevant tickets?
Screenshots or screencasts (if UI/UX change)
Questions:

Does the docs need an update?
Are there any security concerns?
Do we need to update engineering / success?

ran it and it works!


generate dataset

when generateing the data sets seems like its taking a very long time. not sure if it actually completing Repo card metadata block was not found. Setting CardData to empty.
WARNING:huggingface_hub.repocard:Repo card metadata block was not found. Setting CardData to empty.
Repo card metadata block was not found. Setting CardData to empty.
WARNING:huggingface_hub.repocard:Repo card metadata block was not found. Setting CardData to empty.
NcbiDisease-NER-dev: 100%|███████████████████| 924/924 [00:03<00:00, 238.68it/s]
NcbiDisease-NER-test: 100%|██████████████████| 941/941 [00:04<00:00, 196.24it/s]
BC5CDR-NER-train-0:  13%|██▌                | 609/4561 [00:03<00:30, 131.50it/s]
BC5CDR-NER-train-0:  38%|██████▊           | 1715/4561 [00:09<00:19, 143.23it/s]
BC5CDR-NER-train-0:  61%|███████████       | 2796/4561 [00:14<00:07, 227.09it/s]
BC5CDR-NER-test:  27%|█████▌               | 1272/4798 [00:09<00:27, 126.57it/s]
BC5CDR-NER-test:  39%|████████▎            | 1890/4798 [00:14<00:23, 124.91it/s]
BroadTwitter-NER-dev:  59%|█████████▍      | 1188/2002 [00:10<00:07, 115.30it/s]
BroadTwitter-NER-dev:  88%|██████████████  | 1755/2002 [00:15<00:02, 114.49it/s]
BroadTwitter-NER-dev: 100%|████████████████| 2002/2002 [00:17<00:00, 115.12it/s]
WNUT17-NER-test: 100%|██████████████████████| 1287/1287 [00:17<00:00, 71.52it/s]
BC5CDR-NER-train-0: 100%|██████████████████| 4561/4561 [00:22<00:00, 199.49it/s]
WNUT17-NER-train-0: 100%|██████████████████| 3394/3394 [00:25<00:00, 133.29it/s]
BroadTwitter-NER-test: 100%|███████████████| 2002/2002 [00:13<00:00, 146.04it/s]
CoNLL03-NER-test: 100%|█████████████████████| 3453/3453 [00:34<00:00, 99.02it/s]
BC5CDR-NER-train-0:  72%|████████████▉     | 3290/4561 [00:17<00:05, 223.53it/s]
BC5CDR-NER-train-0:  75%|█████████████▌    | 3434/4561 [00:17<00:05, 224.29it/s]
BC5CDR-NER-train-0: 100%|█████████████████▉| 4545/4561 [00:22<00:00, 222.88it/s]
BC5CDR-NER-test: 100%|█████████████████████| 4798/4798 [00:36<00:00, 130.22it/s]
FabNER-NER-dev: 100%|███████████████████████| 2183/2183 [00:46<00:00, 46.57it/s]
BC5CDR-NER-train-24: 100%|█████████████████| 4561/4561 [00:25<00:00, 175.85it/s]
WNUT17-NER-train-24: 100%|█████████████████| 3394/3394 [00:29<00:00, 115.00it/s]
BC5CDR-NER-test:  90%|██████████████████▉  | 4341/4798 [00:34<00:03, 122.35it/s]
BC5CDR-NER-test:  97%|████████████████████▍| 4660/4798 [00:36<00:00, 208.44it/s]
... (more hidden) ...
BC5CDR-NER-train-24:  42%|███████          | 1909/4561 [00:14<00:18, 141.97it/s]
BC5CDR-NER-train-24:  89%|███████████████  | 4048/4561 [00:23<00:02, 228.24it/s]
BC5CDR-NER-train-42: 100%|█████████████████| 4561/4561 [00:20<00:00, 221.22it/s]
BC5CDR-NER-train-42:  28%|████▊            | 1276/4561 [00:05<00:14, 233.07it/sRepo card metadata block was not found. Setting CardData to empty.02, 235.46it/s]
WARNING:huggingface_hub.repocard:Repo card metadata block was not found. Setting CardData to empty.ain-0:  46%|█████▍      | 2431/5342 [00:15<00:23, 125.62it/s]
BroadTwitter-NER-train-0:  74%|████████▉   | 3979/5342 [00:28<00:11, 113.66it/s]
BC5CDR-NER-train-42: 100%|████████████████▉| 4559/4561 [00:20<00:00, 228.19it/s]
BroadTwitter-NER-train-0:  79%|█████████▍  | 4219/5342 [00:30<00:10, 112.01it/s]
BroadTwitter-NER-train-0: 100%|████████████| 5342/5342 [00:36<00:00, 146.11it/s]
WNUT17-NER-train-42: 100%|█████████████████| 3394/3394 [00:30<00:00, 111.55it/s]
FabNER-NER-test: 100%|██████████████████████| 2064/2064 [00:37<00:00, 54.63it/s]
BroadTwitter-NER-train-0:  96%|███████████▍| 5114/5342 [00:35<00:01, 197.95it/s]
BC5CDR-NER-dev:  10%|██▎                    | 456/4582 [00:03<00:34, 120.66it/s]
CoNLL03-NER-train-0: 100%|███████████████| 14041/14041 [01:33<00:00, 149.84it/s]
WNUT17-NER-dev: 100%|██████████████████████| 1009/1009 [00:08<00:00, 124.45it/s]
BC5CDR-NER-dev: 100%|██████████████████████| 4582/4582 [00:27<00:00, 166.55it/s]
BroadTwitter-NER-train-24: 100%|███████████| 5342/5342 [00:25<00:00, 213.61it/s]
BroadTwitter-NER-train-42: 100%|███████████| 5342/5342 [00:24<00:00, 218.66it/s]
CoNLL03-NER-train-24: 100%|██████████████| 14041/14041 [01:20<00:00, 174.10it/s]
CoNLL03-NER-train-42: 100%|██████████████| 14041/14041 [01:19<00:00, 176.13it/s]
CoNLL03-NER-dev: 100%|█████████████████████| 3250/3250 [00:20<00:00, 156.14it/s]
OntoNotes5-NER-test: 100%|████████████████| 12217/12217 [04:51<00:00, 41.97it/s]
MultiNERD-NER-test: 100%|█████████████████| 32908/32908 [10:59<00:00, 49.93it/s]
NcbiDisease-NER-train-0: 100%|█████████████| 5433/5433 [00:15<00:00, 351.93it/s]
NcbiDisease-NER-train-24: 100%|████████████| 5433/5433 [00:15<00:00, 352.46it/s]
NcbiDisease-NER-train-42: 100%|████████████| 5433/5433 [00:15<00:00, 351.97it/s]
BC5CDR-NER-dev:  78%|█████████████████     | 3554/4582 [00:22<00:05, 194.35it/s]
BC5CDR-NER-dev:  83%|██████████████████▏   | 3781/4582 [00:23<00:03, 221.54it/s]
BC5CDR-NER-dev: 100%|█████████████████████▉| 4569/4582 [00:27<00:00, 211.22it/s]
BroadTwitter-NER-train-24:  81%|████████▉  | 4352/5342 [00:20<00:05, 191.82it/s]
BroadTwitter-NER-train-24:  95%|██████████▍| 5067/5342 [00:23<00:01, 187.31it/s]
BroadTwitter-NER-train-24: 100%|██████████▉| 5327/5342 [00:24<00:00, 205.77it/s]
BroadTwitter-NER-train-42: 100%|███████████| 5342/5342 [00:24<00:00, 202.92it/s]

Hi @jmanhype!
Generating the training dataset involves transforming each example into our code-based template and then applying the noising algorithms. Since we have many datasets, some of which are very large, and the process must be done 3 times (once for each pre-computed epoch), it takes a long time. This is expected. We will use all the CPU cores available. On our server with 64 cores, generating the dataset takes approximately 3 hours.  If you have many cores available the tqdm output can be a little confusing.  If you're uncertain whether the process is still ongoing, press enter several times to ensure the remaining tqdm progress bars write on a new line.
If you just want to run the evaluation, you can delete the train_file and dev_file lines in the configs/data_configs files. Generating the test split should be very fast (under 1 hour).


Support Downward api as template variable

It is sometimes useful for acorn containers to have information about themselves.
Also, acorn app to have its information like appInstance name, namespace etc.

Description
There may be cases where we'd like containers to have access to pod information through environment variables.
Some example pod information that can be pulled through the downward api is the fieldRef and resourceFieldRef.
A short list of available fields

field
description

metadata.name
the pod's name

metadata.namespace
the pod's namespace

metadata.uid
the pod's unique ID

metadata.annotations['<KEY>']
the value of the pod's annotation named  (for example, metadata.annotations['myannotation'])

metadata.labels['<KEY>']
the text value of the pod's label named  (for example, metadata.labels['mylabel'])

spec.serviceAccountName
the name of the pod's service account

spec.nodeName
the name of the node where the Pod is executing

status.hostIP
the primary IP address of the node to which the Pod is assigned

status.podIP
the pod's primary IP address (usually, its IPv4 address)

Resources
Kubernetes Downward API: https://kubernetes.io/docs/concepts/workloads/pods/downward-api/
Example Kubernetes downward API usage: https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/
Previous discussion in Acorn Slack: https://acorn-users.slack.com/archives/C03R9ME0SKC/p1663138625938179


whitelist/blacklist function

Is your feature request related to a problem? Please describe.
Some items already created in Itemsadder are not suitable for setting expiration.
Describe the solution you'd like
Need whitelist/blacklist for foods with expiration function.
Describe alternatives you've considered

enable-fresh-food-expiration: false

I disabled the expiration for all fresh food because it is not possible to disable it for only tropical fish custom data items.
However, this solution does not enable the essential functionality of the plugin, so I am hesitant to deploy this plugin on my server.
Additional context
It would be great if it could be specified whether or not the expiration feature is allowed by specifying the data number of the custom model.

If I understand correctly, you'd like to be able to define a list of items for which you don't want expiration system ?

So the item in the list with this custom model data would not be affected by the expiration system ?

Yes! like that!

Perfect, I'm working on it today

A functional blacklist has been implemented in version 0.0.99 of NoMoreCooked, available on Spigot.
Here's how it looks, and its position in the configuration (just below itemsAdder support activation)

Have a nice day !

Thank you. I tried it immediately.
Unfortunately, it does not seem to be working.
For example, my certain Itemsadder Item (custom fishing fish) is assigned to TROPICAL_FISH:10000.
I have listed it on my Blacklist. When I actually caught the fish, the expiration was displayed in Lore.
Could this be a compatibility issue with the custom fishing plugin?
LiteFish-4.7.0
NoMoreCooked-0.0.99-RELEASE
Paper 1.21

I'm sorry to hear that there's a problem.
However I don't think it's due to a compatibility problem between plugins, I think rather that the method called doesn't go through a blacklist check.
Could you run the following test :

Take one of your fish added to the blacklist with the IA menu or via the /iaget command so that the fish is not affected by the expiration system
Throw the fish on the ground
Pick up the fish again
Check if the fish is affected by the expiry system once picked up

Thanks in advance

Thank you for your replayThanks for the reply.
I have tried 1 to 4.
Items were not affected by the expiration date system.
More to the point, foods that were not blacklisted were also not affected by the expiration date system.

So there's a problem specific to your environment.
In the tests I've just redone, the blacklist works correctly for item pickup.
I've also tested with litefish, and the blacklist works correctly.
For problem number one, which concerns the blacklist not working when you pick up an item, could you share your NoMoreCooked config.yml?
For problem number two, which concerns litefish, I think the problem comes from the fact that when you added your fish via the GUI, it was subject to an expiry date, so now every time the fish is caught, it has an expiry date. At least, that's what I noticed during my tests. I invite you to replace your fish with virgin fish that don't have expiry dates and to repeat the tests.
It's also possible that problem number two is linked to problem number one, so I'm waiting for your configuration file so that I can go further in detecting the problem.
What's reassuring is that I've come up with a positive result for both, but I've also gone through a negative result for both, so a solution is possible.

Regarding the second issue you mentioned, I don't think it is possible. I think it is unlikely, because I have been using litefish since before the NMC was installed. And now, encountering the NMC and trying to install it, I faced this problem for the first time.
Anyway, I am a little relieved that there is hope for a solution.
The config file is attached.
Please check it.
config.txt

For the blacklist, I see that you haven't activated it. You need to change enable-blacklist: false to enable-blacklist: true. I've done it for you.
You've also deactivated enable-fresh-food-expiration, which explains why other foods when you pick them up are not affected. I've changed it to true, you're free to deactivate it again, but if I understand correctly, you want it activated.
Another thing, I see that you've added # in front of some of the items in the fresh foods list, I'd recommend that you put -1 like I did to disable expiration for an item. You can set an expiration time if you want expiration to apply to these items, which I'm assuming you want to do given the blacklist. So I've just set an expiry time for TROPICAL_FISH so that you can see that the blacklist works.
I can also see that the configuration is a bit damaged after the config update, so I've redone a clean one for you, keeping your settings (except those mentioned above) but putting some options in the right place.
config.txt
Let me know if this solves your problems.

Thank you very much.
I seem to have sent you a configuration file during various trial and error attempts. Sorry about that.
I tried with the configuration file you shared me, but things are still the same. It seems that there is a conflict with some other plugin.
-1! I didn't know that was possible. To be honest, I thought that would be fine for a while.
In any case, I'll try to find time to see what it's conflicting with.

Okay, because the configuration sent should indeed work.
As a further step towards defining the cause of the problem, you can simply leave NoMoreCooked, LiteFish and ItemsAdder on the server and run the tests to ensure that the error is no longer configuration-related.
If everything works as expected, you can add others plugins as you go along to find out which one is responsible for the conflict, and then either contact the developer of the conflicting plugin yourself, or give me the information so I can work out a solution with the developer.
If you have any console errors, please let me know.
For the -1, it's a logic I've implemented, I don't recommend this operation for all other plugins not knowing how the plugin would react.
In any case, I'll be happy to help if you need any further assistance in the future.

I tried running the server with your modified configuration file and the minimum configuration plugin.
However, it still shows the expiration date.
Next, I replaced the litefish fish again with one obtained from /iaget.
The expiration date still appears on the fish.
The expiration date is displayed when I drop the material obtained with /iaget on the ground. This material custom model data is blacklisted. Is this behavior correct?
I think that the expiration date should not be displayed since it is in the blacklist...
Server

paper 1.21

Installed plugins

ItemsAdder_4.0.2-beta-release-12
LiteFish-4.7.1
LoneLibs
NoMoreCooked-0.0.99-RELEASE
ProtocolLib

config.txt

Indeed, you're right, the configuration I sent you doesn't behave as expected. I've just used it in the same environment as you, and the problem occurs.
Yesterday I managed to solve the problem, so I'm trying to find out which option in the configuration breaks everything, and I'll get back to you as soon as I've found a solution.

I found the problem, the configuration had nothing to do with it, it was in fact a problem with the format of the Blacklist, it could only handle one custom model data per item, now it can handle as many as required.
Update 0.1 of NoMoreCooked, to be released shortly, will implement the fix.

Thank you very much.
I look forward to trying 0.1.
Also, apart from this, I've been touching around with the plugin and have come up with a few ideas for features that you might be interested in.
I will discuss them with you again at some point.

Version 0.1 of NoMoreCooked has been released on Spigot with new features and the blacklist fix, so let me know if the blacklist issue has been resolved for you !
As for ideas for improving the plugin, I'm open to all proposals, and I appreciate your willingness to improve it

I tried 0.1.
It seems to be working well!
I tried 0.1.
It seems to be working well!
My only concern is that there was an error in the process of converting the data to the new format.
I checked and found that the conversion was not successful in the following areas
old file
# Date and time format in item lore
date-format: "yyyy年MM月dd日HH:mm" # Preferred date format
hour-format: "HH:MM:SS" # Preferred time format

new file
# Date and time format in item lore
date-format: "yyyy年MM月dd日HH
hour-format: "HH:mm:ss" # Preferred time format

This may have happened because HH:mm was not assumed as the date-format.

That's because it had to recover the colon of the value as a key-value separator and it didn't handle the case well because it wasn't foreseen in the auto-conversion method
I had anticipated the hour-format but not the date-format
I'm going to remove the deletion of the old configuration after conversion to avoid losing the old values if this happens
I'm also going to ignore the date-format for the moment in the automatic conversion
Version 0.1.1 implements the above elements
Glad to hear the blacklist now works for you !


Comma-separated list of values

This PR sets comma (which better fits here) as a separator to ALLOWED_HOSTS and INTERNAL_IPS env variables and makes heroku doc  consistent with the code.

Codecov Report

Merging #1221 into master will decrease coverage by 0.14%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1221      +/-   ##
==========================================
- Coverage   70.22%   70.07%   -0.15%
==========================================
  Files         117      117
  Lines        6314     6316       +2
  Branches      752      752
==========================================
- Hits         4434     4426       -8
- Misses       1716     1725       +9
- Partials      164      165       +1

Impacted Files
Coverage Δ

saleor/settings.py
89.51% <100%> (+0.17%)
:arrow_up:

saleor/core/templatetags/shop.py
80.95% <0%> (-9.53%)
:arrow_down:

saleor/dashboard/templatetags/utils.py
87.09% <0%> (-6.46%)
:arrow_down:

saleor/core/utils/__init__.py
75% <0%> (-2.09%)
:arrow_down:

saleor/product/utils.py
85.29% <0%> (-1.18%)
:arrow_down:

saleor/core/utils/random_data.py
80.82% <0%> (-0.76%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8acdd4b...a3ae77e. Read the comment docs.


Add an ignore_annotations option for unit testing

This changes the behavior of promtool so that any <alert> that has no exp_annotations field set and that is part of a <test_group> or an <alert_test_case> that has ignore_annotations set to true will not have annotations checked as part of the test.
The idea is that while it’s generally a good idea to test that both labels and annotations have expected values, it is sometimes impractical to do so, for instance if you have an annotation that contains the value of the signal that triggered an alert or if you have stuff that is very verbose and you just end up copying and pasting things from the alert definition to the test file, which is annoying and kind of defeats the purpose of the test.
Of course, we keep the existing behavior as the default. Thoughts?

Thanks a lot for opening the PR and using the testing framework, but sadly I agree with Brian and Julien. We looked at this in the bug scrub. With the deterministic inputs, the output should be deterministic as well, so we don't see how the annotations could change between runs. Could you give us an instance where this could be the case and we can re-open this once we have the use-case.


[4.1] Fix option order in select

REFS: #2847

A new inspection was created.

The inspection completed: No new issues

Thanks for the talk. Note - how about if we populate the <options> with PHP, instead of JS? Like we do in select2 and select2_multiple? Would that be an alternative solution?
If not, this is a clean solution too, we'll merge as-is.

A new inspection was created.

A new inspection was created.

ping @tabacitu
Removed js options, we get them directly from php. No more order worries.

The inspection completed: No new issues

The inspection completed: No new issues

Test inside repeatable.

A new inspection was created.

@tabacitu merge please. Works good inside repeatable and inline.

The inspection completed: No new issues


Typo in retires vs retries

Not a bug perse, but I noticed that the documentation on the client.py ( for example here https://github.com/splunk/splunk-sdk-python/blob/master/splunklib/client.py#L322 but also on the other lines in the code) and other source code files contains the keyword retires as an input param, although I believe it should be retries, correct?
Because, later in the code, the keyword retries is used and not the keyword retires as it is not mentioned afterwards anywhere esle just in the docs of classes/methods.

Fix in PR: https://github.com/splunk/splunk-sdk-python/pull/597


[CLOSED] Medical safety alerts

 Issue by tommyp
Friday 15 August 2014 at 16:21 GMT
Originally opened as https://github.com/alphagov/specialist-publisher/pull/231

Just like all the other Finders this PR does the following:

add Medical Safety Alerts schema
add model, builder and validator for MSA
add formatters
permission checks
add controllers
add observers and services
adding views
testing MSA workflow

This relies on https://github.com/alphagov/govuk_content_models/pull/218. Ticket.
UPDATE: Panopticon update PR found here:
https://github.com/alphagov/panopticon/pull/195

tommyp included the following code: https://github.com/alphagov/specialist-publisher/pull/231/commits

 Comment by tommyp
Monday 18 August 2014 at 08:14 GMT

This actually needs a bit of work once https://github.com/alphagov/specialist-publisher/pull/230 gets merged as there will be some conflicts. Namely the views need removed, the form refactored and the config/routes.rb needs to be changed to follow the new DRY approach.

 Comment by tommyp
Monday 18 August 2014 at 10:26 GMT

Did I accidentally have Drug Safety Update in this PR or are you referencing the fact that we have 2 similarly named Finders?

 Comment by bestie
Monday 18 August 2014 at 10:08 GMT

This looks good other than mixed use of "medical safety alert" / "drug safety update" is there a logic behind this? This could easily become confusing.

 Comment by bestie
Monday 18 August 2014 at 10:33 GMT

Yes @AliceNewton just explained the difference to me and I was about to highlight those

 Comment by bestie
Monday 18 August 2014 at 12:29 GMT

@tommyp This doesn't seem dependent on #230 am I OK to merge?

 Comment by Themitchell
Monday 18 August 2014 at 15:03 GMT

Updated Panopticon to new models gem here:
https://github.com/alphagov/panopticon/pull/195

 Comment by alicebartlett
Tuesday 19 August 2014 at 09:56 GMT

Waiting on a deploy from panopticon before we merge this (slot booked for 1.30 19-aug). Merge this after that has gone out.

 Comment by bestie
Tuesday 19 August 2014 at 16:10 GMT

Dependencies are satisfied but we're waiting for drug safety updates to be deployed first.

 Comment by Themitchell
Wednesday 27 August 2014 at 10:46 GMT

I have added a couple of commits to deal with multi selects. Our new testing strategy should pick this up in future.


Add ability to restart the search

This adds the ability for the MCTS search algorithm to perform a (soft)
restart.  The soft restart erases information related to the evaluation
statistics (such as average runtime or top-k best runs).
Some information carries over across restarts, most notably:

The current cut
Nodes which were found dead through backtracking
Performance model bounds

The results of constraint propagation are not kept; however, the
structure of the tree ensures that we won't perform contraint
propagation if the result has already failed (or was cut due to the
performance model) in a previous restart.

bors r=andidr


react/sort-comp breaks property reference

import _ from 'lodash'
import React from 'react'

class Test extends React.PureComponent {
  fetch = async () => {}

  lazyFetch = _.debounce(this.fetch, 1000)
}

It will report an error now, but if we change the order, fetch will be undefined on lazyFetch initialize.

This is invalid anyways - it's this.fetch. fetch is a global variable here.

(iow, class fields become own properties on this - they're not lexical variables)

@ljharb I just forgot to add this. in my example, although it doesn't change anything.

This is true. However, I'd say that if you have a class field that depends on other ones, it belongs in the constructor, and not as a class field.
Does sort-comp autofix here? If so, it's a bug. If not, it's a case for an override comment.

Sorry I don't understand what does Does sort-comp autofix here? mean here, it throws an error, of course it can't be autofixed.

That’s not what i mean - i mean that is the rule autofixing your code such that it starts throwing an error? Or is only your manual fix what introduces the error?

I can only manually change the sorts to prevent eslint error from import/sort-comp, and then it throws an error from lodash.debounce about undefined function at runtime.

Gotcha. I think this is a case for an eslint override comment, or better, the lazyFetch class field belongs instead in the constructor.

Do you mean it is a wont't fix for eslint-plugin-react?

Yes, I mean that I don't think it's a problem that needs fixing. Class fields shouldn't depend on each other like that.


Add handleChangePassword functionality to User Settings

Can you tell me more about your reasoning to use login endpoint to verify the user's password?

LGTM!


APT safelist request for singular in trusty

Actually, my mistake, I did not realize this package was only added in Ubuntu 16.04.


Item list changed before orderChanged triggered

HI, I have Order property for each item; changed when item moved, then sync the change to the server.
As you see here  http://plnkr.co/edit/wKDy3meQvFoMSlrO91tK?p=preview  (Try randomly reorder the list many times)
You will notice that the order duplicated and some items override others item in the list. It cause duplicates in the repeater  and dismiss the order.
is this known issue or i do it wrong?

@asolimaan You need to remove the filter orderBy: 'Order', why do you need this? don't you need to retain the order that you dragged.

Yes i need to retain the order i dragged, but i need to save that order too in db to show the list in other pages with same order, in addition i want to add label before the item to show its new Order after dragging.
I updated plunkr to simulate my case exactly after remove Order filter.
The exceptions is gone but the list still not ordered.
http://plnkr.co/edit/wKDy3meQvFoMSlrO91tK?p=preview
my use case as follow :

First item
Second item
Last Item
after moving last item to the top, it becomes:
Last Item
First Item
Second Item


[solidjs-tailwind] Clear "X" on filter dropdowns is not currently operational

Description

when clicking the X in a filter dropdown nothing happens, and no currently selected filters are cleared. Expected behavior is that currently selected filters would be cleared when the X is clicked

I second this and will close the issue


Support 'x-queue-mode' option in config

It would be nice have arguments like x-queue-mode for queue properties.

Fixed in 0d5ea17


MODEL accelerating .TensorRT

did anyone try to accelerate the mmd models ， transform it into tensorrt fp 32 or 16 or int8 ?
if you've done related work can we discuss some problems about it?

You can convert your model from PyTorch to ONNX and then convert ONNX to TensorRT engine. There would be some ops that are not supported in TensorRT. You may have to write your custom TensorRT plugin. BTW, welcome to contribute your custom TensorRT plugin to mmcv.

You can convert your model from PyTorch to ONNX and then convert ONNX to TensorRT engine. There would be some ops that are not supported in TensorRT. You may have to write your custom TensorRT plugin. BTW, welcome to contribute your custom TensorRT plugin to mmcv.


Brand @Contoso template failed.

Here is the exception that occurred:

Type = Microsoft.SharePoint.Client.ServerException Message = Value cannot be null. Parameter name: s Source = Microsoft.SharePoint.Client.Runtime ServerStackTrace = ServerErrorCode = -2147467261 ServerErrorTypeName = System.ArgumentNullException ServerErrorTraceCorrelationId = becddc9e-607a-8000-8836-fd4134a9e8cb TargetSite = Void ProcessResponseStream(System.IO.Stream) StackTrace = at Microsoft.SharePoint.Client.ClientRequest.ProcessResponseStream(Stream responseStream) at Microsoft.SharePoint.Client.ClientRequest.ProcessResponse() at Microsoft.SharePoint.Client.ClientRequest.d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.SharePoint.Client.ClientRequest.d__0.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.SharePoint.Client.ClientRuntimeContext.d__0.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.SharePoint.Client.ClientContext.d__4.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.SharePoint.Client.ClientContextExtensions.d__7.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.SharePoint.Client.ClientContextExtensions.ExecuteQueryRetry(ClientRuntimeContext clientContext, Int32 retryCount, Int32 delay, String userAgent) at OfficeDevPnP.Core.Framework.Provisioning.ObjectHandlers.ObjectContentType.CreateContentType(Web web, ProvisioningTemplate template, ContentType templateContentType, TokenParser parser, FileConnectorBase connector, PnPMonitoredScope scope, List1 existingCTs, List1 existingFields, Boolean isNoScriptSite) at OfficeDevPnP.Core.Framework.Provisioning.ObjectHandlers.ObjectContentType.ProvisionObjects(Web web, ProvisioningTemplate template, TokenParser parser, ProvisioningTemplateApplyingInformation applyingInformation) at OfficeDevPnP.Core.Framework.Provisioning.ObjectHandlers.SiteToTemplateConversion.ApplyRemoteTemplate(Web web, ProvisioningTemplate template, ProvisioningTemplateApplyingInformation provisioningInfo, Boolean calledFromHierarchy, TokenParser tokenParser) at OfficeDevPnP.Core.Framework.Provisioning.ObjectHandlers.ObjectHierarchySequenceSites.ProvisionObjects(Tenant tenant, ProvisioningHierarchy hierarchy, String sequenceId, TokenParser tokenParser, ProvisioningTemplateApplyingInformation applyingInformation) at OfficeDevPnP.Core.Framework.Provisioning.ObjectHandlers.SiteToTemplateConversion.ApplyProvisioningHierarchy(Tenant tenant, ProvisioningHierarchy hierarchy, String sequenceId, ProvisioningTemplateApplyingInformation provisioningInfo) at Microsoft.SharePoint.Client.TenantExtensions.ApplyProvisionHierarchy(Tenant tenant, ProvisioningHierarchy hierarchy, String sequenceId, ProvisioningTemplateApplyingInformation applyingInformation) at SharePointPnP.ProvisioningApp.WebJob.ProvisioningFunction.d__2.MoveNext() in C:\github\sp-provisioning-service\SharePointPnP.ProvisioningApp\SharePointPnP.ProvisioningApp.WebJob\ProvisioningFunction.cs:line 360 HResult = -2146233088
Category

[ ] Question
[ ] Typo
[x] Bug
[ ] Additional article idea

Expected or Desired Behavior
Site should be created as mentioned on the following link:
https://provisioning.sharepointpnp.com/home/details?packageId=c26dac20-7177-4bbc-9ed0-dcf6b7b529f1
Steps to Reproduce
Go to https://provisioning.sharepointpnp.com/
Click on Brand @ Contoso template
Click Add to Tenant
Click Provision
Click Confirm in the pop-up confirmation screen
Wait for the email after the provision#### Submission Guidelines

Hi @bhupendrasingh282,
Thanks for the feedback. Are you still experiencing this issue?

Hi @bhupendrasingh282 ,
I don't see any reply on this thread, so I will close this item. If you are still experiencing the issue let me know and I will be more than happy to open it back. Thanks.