Re: [LyX/master] Correctly track ulem commands with change tracking
Le 24/10/2016 à 01:00, Enrico Forestieri a écrit : On Sun, Oct 23, 2016 at 11:52:55PM +0200, Guillaume Munch wrote: The code does look fragile to me. I do not think that asking that developers care about maintainability is being overzealous. Then, maybe I am mistaken about the code and you got to something found maintainable enough after thinking about it a lot, such that you did not feel the need asking for advice. In that case, maybe all the misunderstandings comes indeed from a different appreciation of what effort is asked. But to know this we would need to speak about the code. Every time I want to discuss your commits, I know that you are going to take things personally, to the point that sometimes I just prefer to let it off before even asking. I do not know another LyX developer who reacts like you do. It's not what you say but how you say it. Before arguing that I am not the official maintainer, ask yourself who was the only one who tested several of your big changes and spent a lot of time writing detailed testcases (lots of them!)? And where would LyX be if I had not? I think you have an enormous ego and presumptuous manners. I have nothing more to add to this discussion. Good bye. Yeah, I see how this comes across, this is not what I meant, does not correspond to what I think, and I regret writing this. The discussion went nowhere I wanted and I regret starting it. I will no longer send such messages to the list in the future.
Re: [LyX/master] Correctly track ulem commands with change tracking
On Sun, Oct 23, 2016 at 11:52:55PM +0200, Guillaume Munch wrote: > > The code does look fragile to me. I do not think that asking that > developers care about maintainability is being overzealous. Then, maybe > I am mistaken about the code and you got to something found maintainable > enough after thinking about it a lot, such that you did not feel the > need asking for advice. In that case, maybe all the misunderstandings > comes indeed from a different appreciation of what effort is asked. > > But to know this we would need to speak about the code. Every time I > want to discuss your commits, I know that you are going to take things > personally, to the point that sometimes I just prefer to let it off > before even asking. I do not know another LyX developer who reacts like > you do. It's not what you say but how you say it. > Before arguing that I am not the official maintainer, ask yourself who > was the only one who tested several of your big changes and spent a lot > of time writing detailed testcases (lots of them!)? And where would LyX > be if I had not? I think you have an enormous ego and presumptuous manners. I have nothing more to add to this discussion. Good bye. -- Enrico
Re: [LyX/master] Correctly track ulem commands with change tracking
Le 23/10/2016 à 22:53, Enrico Forestieri a écrit : On Sun, Oct 23, 2016 at 07:02:31PM +0200, Guillaume Munch wrote: Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b Author: Enrico Forestieri Date: Sun Oct 23 18:23:41 2016 +0200 Correctly track ulem commands with change tracking LyX assumes that everything in \lyxdeleted is struck out by ulem and increases the corresponding counter. However, deleted display math material is struck out using tikz. As we also take into account the deletion of underlined display math (in order to properly position such material vertically), we have to take care that the count is correct. This code (this commit and previous related commits) looks fragile to me. Did you not prefer to present a (full and tested) patch on the list and ask other people about it before committing? Sorry, but I think that your comments are unwarranted and overzealous. Moreover, this is something that you should pretend from a novice. I think I know when something should be submitted for review before committing. And you are not the official maintainer. Please, try to be less off-putting. Thank you. The code does look fragile to me. I do not think that asking that developers care about maintainability is being overzealous. Then, maybe I am mistaken about the code and you got to something found maintainable enough after thinking about it a lot, such that you did not feel the need asking for advice. In that case, maybe all the misunderstandings comes indeed from a different appreciation of what effort is asked. But to know this we would need to speak about the code. Every time I want to discuss your commits, I know that you are going to take things personally, to the point that sometimes I just prefer to let it off before even asking. I do not know another LyX developer who reacts like you do. Before arguing that I am not the official maintainer, ask yourself who was the only one who tested several of your big changes and spent a lot of time writing detailed testcases (lots of them!)? And where would LyX be if I had not? I wish we could speak about the code instead. Guillaume
Re: [LyX/master] Correctly track ulem commands with change tracking
On Sun, Oct 23, 2016 at 07:02:31PM +0200, Guillaume Munch wrote: > Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : > > commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b > > Author: Enrico Forestieri > > Date: Sun Oct 23 18:23:41 2016 +0200 > > > > Correctly track ulem commands with change tracking > > > > LyX assumes that everything in \lyxdeleted is struck out by ulem > > and increases the corresponding counter. However, deleted display > > math material is struck out using tikz. As we also take into > > account the deletion of underlined display math (in order to > > properly position such material vertically), we have to take > > care that the count is correct. > > > This code (this commit and previous related commits) looks fragile to > me. Did you not prefer to present a (full and tested) patch on the list > and ask other people about it before committing? Sorry, but I think that your comments are unwarranted and overzealous. Moreover, this is something that you should pretend from a novice. I think I know when something should be submitted for review before committing. And you are not the official maintainer. Please, try to be less off-putting. Thank you. -- Enrico
Re: [LyX/master] Correctly track ulem commands with change tracking
On Sun, Oct 23, 2016 at 09:09:22PM +0200, Guillaume Munch wrote: > Le 23/10/2016 à 19:55, Richard Heck a écrit : > > On 10/23/2016 01:02 PM, Guillaume Munch wrote: > > > Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : > > > > commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b > > > > Author: Enrico Forestieri > > > > Date: Sun Oct 23 18:23:41 2016 +0200 > > > > > > > > Correctly track ulem commands with change tracking > > > > > > > > LyX assumes that everything in \lyxdeleted is struck out by ulem > > > > and increases the corresponding counter. However, deleted display > > > > math material is struck out using tikz. As we also take into > > > > account the deletion of underlined display math (in order to > > > > properly position such material vertically), we have to take > > > > care that the count is correct. > > > > > > > > > This code (this commit and previous related commits) looks fragile to > > > me. Did you not prefer to present a (full and tested) patch on the list > > > and ask other people about it before committing? > > > > Looked pretty straightforward to me. This is a very common use of an > > OutputParams flag. In any event, I take it that this fixed a bug in some of > > the previous work Enrico did on this. It's not usually our policy to > > require > > discussion of that kind of thing. > > > > Where we do always want discussion is with significant changes of behavior, > > and especially of UI-related changes that affect the user experience. If > > I'm > > remembering correctly, there has been some such discussion around how > > deleted displayed math is handled. > > > > Yes, each of the commits looks straightforward. Some of the flags > were already there indeed. And yes, the change in behaviour has been > discussed and is most welcome. > > I meant something else. It seems that "common use of an OutputParams flag" > results in having the logic of a feature scattered all around in tiny bits > of code. I am worried that this is not going to be easy to > maintain. > > When I make small or large changes to the code I always try to give back > something more modular than what I start with. So I wonder whether this > logic could be centralized in some way. Maybe you should try to understand the code in Paragraph::latex. After I did that, it seemed so straightforward to me to not require further comments. -- Enrico
Re: [LyX/master] Correctly track ulem commands with change tracking
Le 23/10/2016 à 19:55, Richard Heck a écrit : On 10/23/2016 01:02 PM, Guillaume Munch wrote: Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b Author: Enrico Forestieri Date: Sun Oct 23 18:23:41 2016 +0200 Correctly track ulem commands with change tracking LyX assumes that everything in \lyxdeleted is struck out by ulem and increases the corresponding counter. However, deleted display math material is struck out using tikz. As we also take into account the deletion of underlined display math (in order to properly position such material vertically), we have to take care that the count is correct. This code (this commit and previous related commits) looks fragile to me. Did you not prefer to present a (full and tested) patch on the list and ask other people about it before committing? Looked pretty straightforward to me. This is a very common use of an OutputParams flag. In any event, I take it that this fixed a bug in some of the previous work Enrico did on this. It's not usually our policy to require discussion of that kind of thing. Where we do always want discussion is with significant changes of behavior, and especially of UI-related changes that affect the user experience. If I'm remembering correctly, there has been some such discussion around how deleted displayed math is handled. Yes, each of the commits looks straightforward. Some of the flags were already there indeed. And yes, the change in behaviour has been discussed and is most welcome. I meant something else. It seems that "common use of an OutputParams flag" results in having the logic of a feature scattered all around in tiny bits of code. I am worried that this is not going to be easy to maintain. When I make small or large changes to the code I always try to give back something more modular than what I start with. So I wonder whether this logic could be centralized in some way. Guillaume
Re: [LyX/master] Correctly track ulem commands with change tracking
On 10/23/2016 01:02 PM, Guillaume Munch wrote: > Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : >> commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b >> Author: Enrico Forestieri >> Date: Sun Oct 23 18:23:41 2016 +0200 >> >> Correctly track ulem commands with change tracking >> >> LyX assumes that everything in \lyxdeleted is struck out by ulem >> and increases the corresponding counter. However, deleted display >> math material is struck out using tikz. As we also take into >> account the deletion of underlined display math (in order to >> properly position such material vertically), we have to take >> care that the count is correct. > > > This code (this commit and previous related commits) looks fragile to > me. Did you not prefer to present a (full and tested) patch on the list > and ask other people about it before committing? Looked pretty straightforward to me. This is a very common use of an OutputParams flag. In any event, I take it that this fixed a bug in some of the previous work Enrico did on this. It's not usually our policy to require discussion of that kind of thing. Where we do always want discussion is with significant changes of behavior, and especially of UI-related changes that affect the user experience. If I'm remembering correctly, there has been some such discussion around how deleted displayed math is handled. Richard
Re: [LyX/master] Correctly track ulem commands with change tracking
Le 23/10/2016 à 18:38, Enrico Forestieri a écrit : commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b Author: Enrico Forestieri Date: Sun Oct 23 18:23:41 2016 +0200 Correctly track ulem commands with change tracking LyX assumes that everything in \lyxdeleted is struck out by ulem and increases the corresponding counter. However, deleted display math material is struck out using tikz. As we also take into account the deletion of underlined display math (in order to properly position such material vertically), we have to take care that the count is correct. This code (this commit and previous related commits) looks fragile to me. Did you not prefer to present a (full and tested) patch on the list and ask other people about it before committing?