Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[Uwe Stöhr]

El 17.05.2017 a las 16:29, José Abílio Matos escribió:


One of my main interests regarding lyx2lyx is that its code is semantically
meaningful. In this case that means that if the same chunk of code is repeated
over and over we refactor it into a function and give it a meaningful name.


Hello José,

many thanks for your work. All I know of Python is what I stole from 
existing routines. Therefore In only once in the past created an own 
routine:

revert_Argument_to_TeX_brace
convert_TeX_brace_to_Argument
which are used in lyx_2_1.py and lyx_2_2.py. (maybe it is worth it to 
move them to lyx2lyx_tools.py?


Your patch looks good. You are also our python expert so of course 
please put it in - if it will work with python 2 and 3. (I have no time 
right now to try Python 3 with your patch.)


best regards
Uwe

Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[Uwe Stöhr]

El 16.05.2017 a las 00:20, José Abílio Matos escribió:


+document.warning("Malformed LyX document: Missing '\\end_inset' of
Float inset.")


As Guillaume suggested I think that this warning is incorrect.


Yes, it makes no sense here. Seems to be a copy paste mistake that I 
made. I removed it now.



+regexp = re.compile(r'^.*reqno.*')
+i = find_re(document.header, regexp, 0)


IMHO we should left this option to the user as we do in all the other cases.
In this case that would mean removing the last part of the function.


But I was requested to handle leqno/reqno and not leaving it to the user.

I assured now that reqno is only handled if it is a document class 
option. I think it is now OK.


regards Uwe

Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[Richard Heck]

On 05/15/2017 06:20 PM, José Abílio Matos wrote:
> On Saturday, 13 May 2017 19.39.55 WEST Uwe Stöhr wrote:
>> commit 0dd3311dd42996f9aede7d8c58b27937ebb48b54
>> Author: Uwe Stöhr 
>> Date:   Sat May 13 20:39:45 2017 +0200
>>
>> rename buffer parameter math_number_before to math_numbering_side
>>
>> this is a fileformat change
>>
>> also try to fix an UI issue that JMarc gets
>>
>> +def convert_mathnumberingname(document):
>> +" rename the \\math_number_before tag to \\math_numbering_side "
>> +document.warning("Malformed LyX document: Missing '\\end_inset' of
>> Float inset.")
> As Guillaume suggested I think that this warning is incorrect. The function 
> does not check for insets since it works on the header and also this call is 
> unconditional...
>
>> +regexp = re.compile(r'(\\math_number_before 1)')
>> +i = find_re(document.header, regexp, 0)
>> +if i != -1:
>> +document.header[i] = "\\math_numbering_side left"
>> +regexp = re.compile(r'(\\math_number_before 0)')
>> +i = find_re(document.header, regexp, 0)
>> +if i != -1:
>> +document.header[i] = "\\math_numbering_side default"
>> +# check if the document uses the class option "reqno"
>> +k = find_token(document.header, "\\math_numbering_side", 0)
> This part of the code is OK:
>
>> +regexp = re.compile(r'^.*reqno.*')
>> +i = find_re(document.header, regexp, 0)
>> +if i != -1:
>> +document.header[k] = "\\math_numbering_side right"
>> +# delete the found option
>> +document.header[i] = document.header[i].replace(",reqno", "")
>> +document.header[i] = document.header[i].replace(", reqno", "")
>> +document.header[i] = document.header[i].replace("reqno,", "")
>> +j = find_re(document.header, regexp, 0)
>> +if i == j:
>> +# then we have reqno as the only option
>> +del document.header[i]
>> +
>> +
> My objection are on this part of the code. Basically we search for a 
> reference 
> to reqno in the lyx header, that in this case means the latex preamble or the 
> class options to equate the reqno reference to using the reqno option.
>
> It is very easy to produce a false positive by placing in the latex preamble 
> of the document:
>
> % it would be interesting to use reqno for all or some of the equations
>
> Just the mention of reqno is interpreted as transforming the document to use 
> the equation number on the right.
>
> IMHO we should left this option to the user as we do in all the other cases. 
> In this case that would mean removing the last part of the function.

I trust your judgement on this, José.

Richard

Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[José Abílio Matos]

On Saturday, 13 May 2017 19.39.55 WEST Uwe Stöhr wrote:
> commit 0dd3311dd42996f9aede7d8c58b27937ebb48b54
> Author: Uwe Stöhr 
> Date:   Sat May 13 20:39:45 2017 +0200
> 
> rename buffer parameter math_number_before to math_numbering_side
> 
> this is a fileformat change
> 
> also try to fix an UI issue that JMarc gets
> 
> +def convert_mathnumberingname(document):
> +" rename the \\math_number_before tag to \\math_numbering_side "
> +document.warning("Malformed LyX document: Missing '\\end_inset' of
> Float inset.")

As Guillaume suggested I think that this warning is incorrect. The function 
does not check for insets since it works on the header and also this call is 
unconditional...

> +regexp = re.compile(r'(\\math_number_before 1)')
> +i = find_re(document.header, regexp, 0)
> +if i != -1:
> +document.header[i] = "\\math_numbering_side left"
> +regexp = re.compile(r'(\\math_number_before 0)')
> +i = find_re(document.header, regexp, 0)
> +if i != -1:
> +document.header[i] = "\\math_numbering_side default"
> +# check if the document uses the class option "reqno"
> +k = find_token(document.header, "\\math_numbering_side", 0)

This part of the code is OK:

> +regexp = re.compile(r'^.*reqno.*')
> +i = find_re(document.header, regexp, 0)
> +if i != -1:
> +document.header[k] = "\\math_numbering_side right"
> +# delete the found option
> +document.header[i] = document.header[i].replace(",reqno", "")
> +document.header[i] = document.header[i].replace(", reqno", "")
> +document.header[i] = document.header[i].replace("reqno,", "")
> +j = find_re(document.header, regexp, 0)
> +if i == j:
> +# then we have reqno as the only option
> +del document.header[i]
> +
> +

My objection are on this part of the code. Basically we search for a reference 
to reqno in the lyx header, that in this case means the latex preamble or the 
class options to equate the reqno reference to using the reqno option.

It is very easy to produce a false positive by placing in the latex preamble 
of the document:

% it would be interesting to use reqno for all or some of the equations

Just the mention of reqno is interpreted as transforming the document to use 
the equation number on the right.

IMHO we should left this option to the user as we do in all the other cases. 
In this case that would mean removing the last part of the function.

Regards,
-- 
José Abílio

Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[José Abílio Matos]

On Monday, 15 May 2017 12.50.06 WEST Guillaume MM wrote:
> It looks like this warning shows up every time.

Yes. And it is clearly out of place. The function in question is supposed to 
work on the header, and all the calls are like that (I have some reservations 
regarding the last part but they are unrelated to this).

So this seems clearly a case of copy and paste going wrong. :-)
-- 
José Abílio

Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

[Guillaume MM]

Le 13/05/2017 à 20:39, Uwe Stöhr a écrit :

commit 0dd3311dd42996f9aede7d8c58b27937ebb48b54
Author: Uwe Stöhr 
Date:   Sat May 13 20:39:45 2017 +0200

rename buffer parameter math_number_before to math_numbering_side

this is a fileformat change

also try to fix an UI issue that JMarc gets
---
+def convert_mathnumberingname(document):
+" rename the \\math_number_before tag to \\math_numbering_side "
+document.warning("Malformed LyX document: Missing '\\end_inset' of Float 
inset.")


It looks like this warning shows up every time.