Excel-DNA / IntelliSense

Add in-sheet IntelliSense for Excel UDFs
MIT License
170 stars 52 forks source link

Internationalize ListSeparator in FormulaParser.cs #52

Closed lyhnet closed 6 years ago

lyhnet commented 6 years ago

Hi,

Depending on machine setup the function argument list separator can differ. In some regions ";" is used instead of ",". This causes IntelliSense not to step to next argument text.

I notice the use of ',' in various places, but I am not sure which places affect the issue. One of them is,

public static char ListSeparator = ','; but changing only this to

public static char ListSeparator = Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator

does not solve the problem.

govert commented 6 years ago

One needs to be a bit careful - Excel may be configured to use a different separator to the one in the current culture. It can be read via one of the GET.XXX info functions.

lyhnet commented 6 years ago

Are you sure that excel has a custom seeing for list separator. I've been struggling a lot over the years with internationalization in excel, and I think list separator is controlled only from Windows settings. Decimal and thousands separator is another thing.

govert commented 6 years ago

This code is intended to set up the list separator for the formula parser: https://github.com/Excel-DNA/IntelliSense/blob/master/Source/ExcelDna.IntelliSense/IntelliSenseDisplay.cs#L52

I'll have to figure out why it is not working correctly.

govert commented 6 years ago

@lyhnet I tested it now and everything seems to work correctly with a different list separator. Are you checking the current version (v 1.0.6)?

lyhnet commented 6 years ago

Will check when I am back at work. Don't you agree that the list separator is not an Excel setting, but global for Windows? How do you change the list separator in Excel?

govert commented 6 years ago

The list separator that Excel uses might differ from the Windows setting in some cases, e.g. if the Windows thousands separator and list separator are the same. I'm not whether the "Use system separators" setting affects the list separator at all.

lyhnet commented 6 years ago

OK, I can confirm that the version I am using is 1.0.6. Intellisense keeps showing first argument info when I put a ";" which is the correct list separator on my machine. Putting a "," will show the second argument info, but will produce a evaluation error of course.

My findings are;

  1. if windows regional settings different separator for decimal and lists then Excel will inherit.
  2. if Windows Regional Settings as the same separator for decimal and list, Excel will default to ";" when "," is used and "," when "." is used respectively.

My suggestion for the code is (work in all scenarios with ",", ";" and ".";

` string listSeparator;

        if (System.Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator == System.Globalization.CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator)

        {

            if (System.Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator == ".")

            {

            // Excel will use ","

            listSeparator = ",";

        }

            else if (System.Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator == ",")

            {

                // Excel will use ";"

                listSeparator = ";";

            }

            else

            {

                //Default value - dont know what will happen

                listSeparator = ",";

            }

        }

        else

        {

            // Normal behaviour when Regional Settings does not use same symbol for DecimalSep and ListSep.

            listSeparator=System.Globalization.CultureInfo.CurrentCulture.TextInfo.ListSeparator;

        }

            FormulaParser.ListSeparator = Convert.ToChar(listSeparator); 

        _argumentSeparator = Convert.ToChar(listSeparator) + " "; `
govert commented 6 years ago

On my machine the current IntelliSense code works in all cases, including the ';' fallback case. I wonder why your machine is different...

Could you add these two functions to your add-in, and have a look at the results when called from a sheet?

        [ExcelFunction(IsMacroType =true)]
        public static string GetListSeparator()
        {
            string listSeparator = "?";
            object result;
            if (XlCall.TryExcel(XlCall.xlfGetWorkspace, out result, 37) == XlCall.XlReturn.XlReturnSuccess)
            {
                object[,] options = result as object[,];
                if (options != null)
                {
                    listSeparator = (string)options[0, 4];
                }
            }
            return listSeparator;
        }

        public static string GetIntelliSenseVersion()
        {
            return System.Environment.GetEnvironmentVariable("EXCELDNA_INTELLISENSE_ACTIVE_SERVER");
        }
govert commented 6 years ago

@lyhnet Are you able to confirm whether this is still an issue for you?

Everything seems to work right on my machine with version 1.0.6 when changing the separators, including the edge case where the settings clash. But maybe your machine has a different configuration and we can track down the issue.

govert commented 6 years ago

@lyhnet I believe the current implementation behaves correctly Please re-open the issue if you can still reproduce this problem.